[GitHub] [mynewt-nimble] h2zero commented on a change in pull request #790: nimble/store: Fix nimble store behavior when CCCDs exceed static defined limit

2020-04-13 Thread GitBox
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 *) 
>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:
us...@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

2020-04-06 Thread GitBox
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:
us...@infra.apache.org


With regards,
Apache Git Services