You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/03/31 18:49:29 UTC
[GitHub] [mynewt-nimble] prasad-alatkar opened a new pull request #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
prasad-alatkar opened a new pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790
Issue reference: #788
Description: In cases where CCCDs are exceeding MYNEWT_VAL(BLE_STORE_MAX_CCCDS) (overflow event), we can end up in continuous while loop.
Change list:
1. In `ble_gap_unpair_oldest_peer` changed return value from `0` to `BLE_HS_ENOENT` when there is no entry found.
2. Modified `ble_store_util_status_rr` to address overflow event of CCCDs.
Previous to this PR, if CCCDs used to exceed the MAX_CCCDs then in `ble_store_write` call there used to come OVERFLOW event, which used to call `ble_store_delete_oldest_peer`, now here as we do not have any peer to be deleted, we should have identified that we need to make space for CCCDs and not `peer_sec` or `our_sec`. The PR tries to address this issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-609581758
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610629250
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-614727452
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610579980
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610780987
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608547695
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-607031816
@h2zero That is a valid point, will update if I find anything. Maybe @rymanluk can explain it to us.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406222887
##########
File path: nimble/host/include/host/ble_gap.h
##########
@@ -1896,6 +1896,20 @@ int ble_gap_unpair(const ble_addr_t *peer_addr);
*/
int ble_gap_unpair_oldest_peer(void);
+/**
+ * Similar to `ble_gap_unpair_oldest_peer()`, except it makes sure that current
+ * peer is not deleted.
+ *
+ * @param peer_addr Address of the current peer (not to be deleted)
+ *
+ * @return 0 on success;
+ * A BLE host HCI return code if the controller
+ * rejected the request;
+ * A BLE host core return code on unexpected
+ * error.
+ */
+int ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer);
Review comment:
Agreed !!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r405456758
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
I believe that `ble_store_util_delete_peer()` deletes all what we need.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-609581758
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610477940
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_gap.c
<details>
```diff
@@ -5623,8 +5621,7 @@
}
for (i = 0; i < num_peers; i++) {
- if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0)
- {
+ if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0) {
break;
}
}
```
</details>
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -341,7 +341,7 @@
key.cccd.peer_addr = peer_cccd_addrs[i];
for (j = 0; j < num_bonded_peers; j++) {
- if(memcmp(&peer_cccd_addrs[i], &peer_bonded_addrs[j],
+ if (memcmp(&peer_cccd_addrs[i], &peer_bonded_addrs[j],
sizeof(ble_addr_t)) == 0) {
break;
}
@@ -385,7 +385,7 @@
case BLE_STORE_OBJ_TYPE_CCCD:
/* Try to remove unbonded CCCDs first */
if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr))
- == BLE_HS_ENOENT) {
+ == BLE_HS_ENOENT) {
/* No unbonded CCCDs found to delete, try unpairing oldest peer
* except current peer */
return ble_gap_unpair_oldest_except_curr((void *) &event->overflow.value->cccd.peer_addr);
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] h2zero commented on a change in pull request #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
h2zero commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r404497436
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
I believe @rymanluk is correct, if the current peer exceeds the cccd limit it will likely unpair that peer and disconnect. I like the do nothing if full approach here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406032755
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try to remove unbonded CCCDs first */
+ if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
I don't think we need to clean unbodend CCCDs as we do not keep such in the storage. Unless I miss something, but CCCDs are stored only for bonded devices, and once unbonded, we do clean CCCDs. If it does not work like this (I believe it does) then we have problem somewhere else.
In this case we just need to call the new function and that is it. Nothing else is needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r404738151
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
@rymanluk @h2zero , disregard my earlier comment, yes current peer will be unpaired, and we should simply return error in that case.
I was thinking of adding utility function (`ble_store_clean_old_cccd`) which will delete CCCDs of all unbonded peers. This can be called first in case of `CCCD overflow` and if it fails to delete peer then we can continue with modified `ble_gap_unpair_oldest_peer()`. Please let me know your views on it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406226827
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try to remove unbonded CCCDs first */
+ if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
Hi @rymanluk Please let me know if I am missing something. As per my understanding CCCDs for unbonded devices can get stored as `ble_gatts_clt_cfg_access --> ble_store_write_cccd --> ble_store_write(BLE_STORE_OBJ_TYPE_CCCD, store_value)`.
Edit:
Please disregard my earlier comment, missed on `cccd_value.flags == 0` check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r407949256
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try to remove unbonded CCCDs first */
+ if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
@h2zero actually CCCD's do get stored when devices are bonded. That happens after pairing is completed and both devices have bonding flag in set.
If you do see an issue that CCCDs are stored but device is not actually bonded, please report it as an issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403012231
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,13 +299,35 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
case BLE_STORE_OBJ_TYPE_OUR_SEC:
case BLE_STORE_OBJ_TYPE_PEER_SEC:
case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
Review comment:
@rymanluk agreed !! I have made relevant changes. For CCCD, if `ble_gap_unpair_oldest_peer` is unable to delete any peer, we continue to find other peers occupying CCCDs in storage. I have also modified callback function (`ble_store_util_iter_peer_cccd`) to skip the current (i.e. for which CCCDs need to be stored) peer entry.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403511125
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
I was thinking to call here other version of `unpair_oldest` which would take as a parameter `ble_addr_t ` of device which we don't want to unpair.
If the only device which could be unpair is the one provided, then unpair is not done and this function returns error.
In such a case we know that we cannot store CCC and there is nothing we can do more here. All the other code is not needed then.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403840472
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
@rymanluk That is what I have done actually in `ble_store_util_subscribed_cccds` !! What I think is `ble_gap_unpair_oldest_peer` is not anyway going to unpair current peer (the one for which we want to make space), so introducing similar function with `ble_addr_t` as input won't be really that helpful. Please let me know your thoughts on this.
The issue I believe is only confined to CCCDs, so `ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT` will make sure that we do not have any bonded devices and still we are out of storage space, which can be used to conclude that we need to clean old CCCDs (except the current one).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406038133
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -5605,6 +5605,41 @@ ble_gap_unpair_oldest_peer(void)
return 0;
}
+int
+ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer)
+{
+ ble_addr_t oldest_peer_id_addr[MYNEWT_VAL(BLE_STORE_MAX_BONDS)];
+ int num_peers;
+ int rc, i;
+
+ rc = ble_store_util_bonded_peers(
+ &oldest_peer_id_addr[0], &num_peers, MYNEWT_VAL(BLE_STORE_MAX_BONDS));
+ if (rc != 0) {
+ return rc;
+ }
+
+ if (num_peers == 0) {
+ return BLE_HS_ENOENT;
+ }
+
+ for (i = 0; i < num_peers; i++) {
+ if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0) {
Review comment:
Sure !!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608526892
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -326,7 +326,7 @@
case BLE_STORE_OBJ_TYPE_PEER_SEC:
return ble_gap_unpair_oldest_peer();
case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
/* No peer to unpair in overflow event. Here bonds do not
* exist but CCCDs are stored. Need to delete CCCDs of
* other peer address to make space.*/
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] h2zero commented on a change in pull request #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
h2zero commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r407550200
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try to remove unbonded CCCDs first */
+ if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
> I don't think we need to clean unbodend CCCDs as we do not keep such in the storage. Unless I miss something, but CCCDs are stored only for bonded devices, and once unbonded, we do clean CCCDs. If it does not work like this (I believe it does) then we have problem somewhere else.
>
> In this case we just need to call the new function and that is it. Nothing else is needed.
@rymanluk CCCD's do get stored if the devices pair but one or both of them does not accept bonding, so cleaning those up makes sense in this case. Although it would be better if that never happened to begin with.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r403009539
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -5594,7 +5594,7 @@ ble_gap_unpair_oldest_peer(void)
}
if (num_peers == 0) {
- return 0;
+ return BLE_HS_ENOENT;
Review comment:
Done !!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608547695
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406226827
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try to remove unbonded CCCDs first */
+ if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
Hi @rymanluk Please let me know if I am missing something. As per my understanding CCCDs for unbonded devices can get stored as `ble_gatts_clt_cfg_access --> ble_store_write_cccd --> ble_store_write(BLE_STORE_OBJ_TYPE_CCCD, store_value)`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r402775932
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -5594,7 +5594,7 @@ ble_gap_unpair_oldest_peer(void)
}
if (num_peers == 0) {
- return 0;
+ return BLE_HS_ENOENT;
Review comment:
Can we make it as separate patch?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r402780122
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,13 +299,35 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
case BLE_STORE_OBJ_TYPE_OUR_SEC:
case BLE_STORE_OBJ_TYPE_PEER_SEC:
case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
Review comment:
I believe that for CCCD we should have different handling and if we want to unpair older peer we should not unpair peer for which we store CCCD. If there is nothing to unpair just return the error.
In the CCCD event there is and ble_addr we should use to make sure that one is no unpaired.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406030239
##########
File path: nimble/host/include/host/ble_gap.h
##########
@@ -1896,6 +1896,20 @@ int ble_gap_unpair(const ble_addr_t *peer_addr);
*/
int ble_gap_unpair_oldest_peer(void);
+/**
+ * Similar to `ble_gap_unpair_oldest_peer()`, except it makes sure that current
+ * peer is not deleted.
+ *
+ * @param peer_addr Address of the current peer (not to be deleted)
+ *
+ * @return 0 on success;
+ * A BLE host HCI return code if the controller
+ * rejected the request;
+ * A BLE host core return code on unexpected
+ * error.
+ */
+int ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer);
Review comment:
let us call it `ble_gap_unpair_oldest_except` () and change the parameter name to `peer_addr`.
This functionality is not really dedicated only to current device. You might want to have device which you don't want to unpair no matter what :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-614615008
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406022480
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -5605,6 +5605,41 @@ ble_gap_unpair_oldest_peer(void)
return 0;
}
+int
+ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer)
+{
+ ble_addr_t oldest_peer_id_addr[MYNEWT_VAL(BLE_STORE_MAX_BONDS)];
+ int num_peers;
+ int rc, i;
+
+ rc = ble_store_util_bonded_peers(
+ &oldest_peer_id_addr[0], &num_peers, MYNEWT_VAL(BLE_STORE_MAX_BONDS));
+ if (rc != 0) {
+ return rc;
+ }
+
+ if (num_peers == 0) {
+ return BLE_HS_ENOENT;
+ }
+
+ for (i = 0; i < num_peers; i++) {
+ if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0) {
Review comment:
I would use here `ble_addr_cmp()`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-606821185
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -306,31 +306,31 @@
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
- /* No peer to unpair. Here bonds do not exist but CCCDs are
- * stored. Need to delete CCCDs to make space. */
- rc = ble_store_util_subscribed_cccds(&peer_id_addr,
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ /* No peer to unpair. Here bonds do not exist but CCCDs are
+ * stored. Need to delete CCCDs to make space. */
+ rc = ble_store_util_subscribed_cccds(&peer_id_addr,
&num_peers, 1);
- if ((rc != 0) || (num_peers == 0)) {
- return rc;
- }
-
- union ble_store_key key = {0};
- key.cccd.peer_addr = peer_id_addr;
-
- rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
- if (rc != 0) {
- return rc;
- }
+ if ((rc != 0) || (num_peers == 0)) {
+ return rc;
}
- return rc;
-
- default:
- return BLE_HS_EUNKNOWN;
+ union ble_store_key key = {0};
+ key.cccd.peer_addr = peer_id_addr;
+
+ rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
+ if (rc != 0) {
+ return rc;
+ }
+ }
+
+ return rc;
+
+ default:
+ return BLE_HS_EUNKNOWN;
}
case BLE_STORE_EVENT_FULL:
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull
request #790: nimble/store: Fix nimble store behavior when CCCDs exceed
static defined limit
Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r405530825
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
Hi @rymanluk I think for some peers `CCCD` will exist but `peer_sec` and `our_sec` won't exist (Ref: [subscribe-event ble_gatts_clt_cfg_access](https://github.com/apache/mynewt-nimble/blob/9e26fbe97b3d0bb3a22f15ed9c6035be648d1104/nimble/host/src/ble_gatts.c#L782)). So we need mechanism to clean these stored CCCDs, that is why I have introduced `ble_store_clean_old_cccd()` function.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610629250
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r409499230
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -233,13 +233,15 @@ ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try unpairing oldest peer except current peer */
+ return ble_gap_unpair_oldest_except((void *) &event->overflow.value->cccd.peer_addr);
Review comment:
nitpick: (void *) seems to be not needed here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610780987
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-606821185
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -306,31 +306,31 @@
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
- /* No peer to unpair. Here bonds do not exist but CCCDs are
- * stored. Need to delete CCCDs to make space. */
- rc = ble_store_util_subscribed_cccds(&peer_id_addr,
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ /* No peer to unpair. Here bonds do not exist but CCCDs are
+ * stored. Need to delete CCCDs to make space. */
+ rc = ble_store_util_subscribed_cccds(&peer_id_addr,
&num_peers, 1);
- if ((rc != 0) || (num_peers == 0)) {
- return rc;
- }
-
- union ble_store_key key = {0};
- key.cccd.peer_addr = peer_id_addr;
-
- rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
- if (rc != 0) {
- return rc;
- }
+ if ((rc != 0) || (num_peers == 0)) {
+ return rc;
}
- return rc;
-
- default:
- return BLE_HS_EUNKNOWN;
+ union ble_store_key key = {0};
+ key.cccd.peer_addr = peer_id_addr;
+
+ rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
+ if (rc != 0) {
+ return rc;
+ }
+ }
+
+ return rc;
+
+ default:
+ return BLE_HS_EUNKNOWN;
}
case BLE_STORE_EVENT_FULL:
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608445002
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -322,36 +322,36 @@
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- return ble_gap_unpair_oldest_peer();
- case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
- /* No peer to unpair in overflow event. Here bonds do not
- * exist but CCCDs are stored. Need to delete CCCDs of
- * other peer address to make space.*/
- rc = ble_store_util_subscribed_cccds(&peer_id_addr,
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ /* No peer to unpair in overflow event. Here bonds do not
+ * exist but CCCDs are stored. Need to delete CCCDs of
+ * other peer address to make space.*/
+ rc = ble_store_util_subscribed_cccds(&peer_id_addr,
&num_peers, 1,
- (void *) &event->overflow.value->cccd.peer_addr);
- if (rc != 0) {
- return rc;
- } else if (num_peers == 0) {
- return BLE_HS_ENOMEM;
- }
-
- union ble_store_key key = {0};
- key.cccd.peer_addr = peer_id_addr;
-
- rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
- if (rc != 0) {
- return rc;
- }
+ (void *) &event->overflow.value->cccd.peer_addr);
+ if (rc != 0) {
+ return rc;
+ } else if (num_peers == 0) {
+ return BLE_HS_ENOMEM;
}
- return rc;
-
- default:
- return BLE_HS_EUNKNOWN;
+ union ble_store_key key = {0};
+ key.cccd.peer_addr = peer_id_addr;
+
+ rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
+ if (rc != 0) {
+ return rc;
+ }
+ }
+
+ return rc;
+
+ default:
+ return BLE_HS_EUNKNOWN;
}
case BLE_STORE_EVENT_FULL:
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r407949256
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +372,24 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ /* Try to remove unbonded CCCDs first */
+ if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr)) == BLE_HS_ENOENT) {
Review comment:
@h2zero actually CCCD's do get stored when devices are bonded. That happens after pairing is completed and both devices have set bonding flag.
If you do see an issue that CCCDs are stored but device is not actually bonded, please report it as an issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] h2zero commented on issue #790: nimble/store: Fix
nimble store behavior when CCCDs exceed static defined limit
Posted by GitBox <gi...@apache.org>.
h2zero commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-606926563
Glad you built a proper fix and hope it gets merged.
I still question why we store cccd data for devices that aren’t bonded but that’s a separate issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608445002
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -322,36 +322,36 @@
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- return ble_gap_unpair_oldest_peer();
- case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
- /* No peer to unpair in overflow event. Here bonds do not
- * exist but CCCDs are stored. Need to delete CCCDs of
- * other peer address to make space.*/
- rc = ble_store_util_subscribed_cccds(&peer_id_addr,
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ /* No peer to unpair in overflow event. Here bonds do not
+ * exist but CCCDs are stored. Need to delete CCCDs of
+ * other peer address to make space.*/
+ rc = ble_store_util_subscribed_cccds(&peer_id_addr,
&num_peers, 1,
- (void *) &event->overflow.value->cccd.peer_addr);
- if (rc != 0) {
- return rc;
- } else if (num_peers == 0) {
- return BLE_HS_ENOMEM;
- }
-
- union ble_store_key key = {0};
- key.cccd.peer_addr = peer_id_addr;
-
- rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
- if (rc != 0) {
- return rc;
- }
+ (void *) &event->overflow.value->cccd.peer_addr);
+ if (rc != 0) {
+ return rc;
+ } else if (num_peers == 0) {
+ return BLE_HS_ENOMEM;
}
- return rc;
-
- default:
- return BLE_HS_EUNKNOWN;
+ union ble_store_key key = {0};
+ key.cccd.peer_addr = peer_id_addr;
+
+ rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
+ if (rc != 0) {
+ return rc;
+ }
+ }
+
+ return rc;
+
+ default:
+ return BLE_HS_EUNKNOWN;
}
case BLE_STORE_EVENT_FULL:
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-614615008
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk merged pull request #790: nimble/store:
Fix nimble store behavior when CCCDs exceed static defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk merged pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608441257
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -322,34 +322,34 @@
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- return ble_gap_unpair_oldest_peer();
- case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
- /* No peer to unpair in overflow event. Here bonds do not
- * exist but CCCDs are stored. Need to delete CCCDs of
- * other peer address to make space.*/
- rc = ble_store_util_subscribed_cccds(&peer_id_addr,
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ /* No peer to unpair in overflow event. Here bonds do not
+ * exist but CCCDs are stored. Need to delete CCCDs of
+ * other peer address to make space.*/
+ rc = ble_store_util_subscribed_cccds(&peer_id_addr,
&num_peers, 1,
- (void *) &event->overflow.value->cccd.peer_addr);
- if ((rc != 0) || (num_peers == 0)) {
- return rc;
- }
-
- union ble_store_key key = {0};
- key.cccd.peer_addr = peer_id_addr;
-
- rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
- if (rc != 0) {
- return rc;
- }
+ (void *) &event->overflow.value->cccd.peer_addr);
+ if ((rc != 0) || (num_peers == 0)) {
+ return rc;
}
- return rc;
-
- default:
- return BLE_HS_EUNKNOWN;
+ union ble_store_key key = {0};
+ key.cccd.peer_addr = peer_id_addr;
+
+ rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
+ if (rc != 0) {
+ return rc;
+ }
+ }
+
+ return rc;
+
+ default:
+ return BLE_HS_EUNKNOWN;
}
case BLE_STORE_EVENT_FULL:
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608526892
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -326,7 +326,7 @@
case BLE_STORE_OBJ_TYPE_PEER_SEC:
return ble_gap_unpair_oldest_peer();
case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
/* No peer to unpair in overflow event. Here bonds do not
* exist but CCCDs are stored. Need to delete CCCDs of
* other peer address to make space.*/
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610477940
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_gap.c
<details>
```diff
@@ -5623,8 +5621,7 @@
}
for (i = 0; i < num_peers; i++) {
- if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0)
- {
+ if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0) {
break;
}
}
```
</details>
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -341,7 +341,7 @@
key.cccd.peer_addr = peer_cccd_addrs[i];
for (j = 0; j < num_bonded_peers; j++) {
- if(memcmp(&peer_cccd_addrs[i], &peer_bonded_addrs[j],
+ if (memcmp(&peer_cccd_addrs[i], &peer_bonded_addrs[j],
sizeof(ble_addr_t)) == 0) {
break;
}
@@ -385,7 +385,7 @@
case BLE_STORE_OBJ_TYPE_CCCD:
/* Try to remove unbonded CCCDs first */
if ((rc = ble_store_clean_old_cccds((void *) &event->overflow.value->cccd.peer_addr))
- == BLE_HS_ENOENT) {
+ == BLE_HS_ENOENT) {
/* No unbonded CCCDs found to delete, try unpairing oldest peer
* except current peer */
return ble_gap_unpair_oldest_except_curr((void *) &event->overflow.value->cccd.peer_addr);
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-608441257
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### nimble/host/src/ble_store_util.c
<details>
```diff
@@ -322,34 +322,34 @@
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- return ble_gap_unpair_oldest_peer();
- case BLE_STORE_OBJ_TYPE_CCCD:
- if (( rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
- /* No peer to unpair in overflow event. Here bonds do not
- * exist but CCCDs are stored. Need to delete CCCDs of
- * other peer address to make space.*/
- rc = ble_store_util_subscribed_cccds(&peer_id_addr,
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
+ /* No peer to unpair in overflow event. Here bonds do not
+ * exist but CCCDs are stored. Need to delete CCCDs of
+ * other peer address to make space.*/
+ rc = ble_store_util_subscribed_cccds(&peer_id_addr,
&num_peers, 1,
- (void *) &event->overflow.value->cccd.peer_addr);
- if ((rc != 0) || (num_peers == 0)) {
- return rc;
- }
-
- union ble_store_key key = {0};
- key.cccd.peer_addr = peer_id_addr;
-
- rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
- if (rc != 0) {
- return rc;
- }
+ (void *) &event->overflow.value->cccd.peer_addr);
+ if ((rc != 0) || (num_peers == 0)) {
+ return rc;
}
- return rc;
-
- default:
- return BLE_HS_EUNKNOWN;
+ union ble_store_key key = {0};
+ key.cccd.peer_addr = peer_id_addr;
+
+ rc = ble_store_util_delete_all(BLE_STORE_OBJ_TYPE_CCCD, &key);
+ if (rc != 0) {
+ return rc;
+ }
+ }
+
+ return rc;
+
+ default:
+ return BLE_HS_EUNKNOWN;
}
case BLE_STORE_EVENT_FULL:
```
</details>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #790:
nimble/store: Fix nimble store behavior when CCCDs exceed static defined
limit
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#issuecomment-610579980
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406023837
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -5605,6 +5605,41 @@ ble_gap_unpair_oldest_peer(void)
return 0;
}
+int
+ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer)
+{
+ ble_addr_t oldest_peer_id_addr[MYNEWT_VAL(BLE_STORE_MAX_BONDS)];
+ int num_peers;
+ int rc, i;
+
+ rc = ble_store_util_bonded_peers(
+ &oldest_peer_id_addr[0], &num_peers, MYNEWT_VAL(BLE_STORE_MAX_BONDS));
+ if (rc != 0) {
+ return rc;
+ }
+
+ if (num_peers == 0) {
+ return BLE_HS_ENOENT;
+ }
+
+ for (i = 0; i < num_peers; i++) {
+ if (memcmp(curr_peer, &oldest_peer_id_addr[i], sizeof (ble_addr_t)) != 0) {
+ break;
+ }
+ }
+
+ if (i < num_peers) {
Review comment:
I would do it little simpler.
```
if (i >= num_peers) {
return BLE_HS_ENOMEM;
}
return ble_gap_unpair(&oldest_peer_id_addr[i]);
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r406024875
##########
File path: nimble/host/src/ble_gap.c
##########
@@ -5605,6 +5605,41 @@ ble_gap_unpair_oldest_peer(void)
return 0;
}
+int
+ble_gap_unpair_oldest_except_curr(const ble_addr_t *curr_peer)
+{
+ ble_addr_t oldest_peer_id_addr[MYNEWT_VAL(BLE_STORE_MAX_BONDS)];
Review comment:
nitpick: please rename to `peer_id_addrs`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] rymanluk commented on a change in pull request
#790: nimble/store: Fix nimble store behavior when CCCDs exceed static
defined limit
Posted by GitBox <gi...@apache.org>.
rymanluk commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit
URL: https://github.com/apache/mynewt-nimble/pull/790#discussion_r404129458
##########
File path: nimble/host/src/ble_store_util.c
##########
@@ -230,16 +315,43 @@ ble_store_util_delete_oldest_peer(void)
int
ble_store_util_status_rr(struct ble_store_status_event *event, void *arg)
{
+ int rc = BLE_HS_EUNKNOWN;
+ ble_addr_t peer_id_addr;
+ int num_peers;
+
switch (event->event_code) {
case BLE_STORE_EVENT_OVERFLOW:
switch (event->overflow.obj_type) {
- case BLE_STORE_OBJ_TYPE_OUR_SEC:
- case BLE_STORE_OBJ_TYPE_PEER_SEC:
- case BLE_STORE_OBJ_TYPE_CCCD:
- return ble_gap_unpair_oldest_peer();
-
- default:
- return BLE_HS_EUNKNOWN;
+ case BLE_STORE_OBJ_TYPE_OUR_SEC:
+ case BLE_STORE_OBJ_TYPE_PEER_SEC:
+ return ble_gap_unpair_oldest_peer();
+ case BLE_STORE_OBJ_TYPE_CCCD:
+ if ((rc = ble_gap_unpair_oldest_peer()) == BLE_HS_ENOENT) {
Review comment:
@prasad-alatkar are you sure that `ble_gap_unpair_oldest_peer()` will not unpair current peer? For me it looks it will.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services