[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 issue #783: Make ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()

2020-04-13 Thread GitBox
h2zero commented on issue #783: Make ble_gap_rx_l2cap_update_req() behave like 
ble_gap_rx_param_req()
URL: https://github.com/apache/mynewt-nimble/pull/783#issuecomment-612954585
 
 
   @zacwbond What @rymanluk is saying is that the procedure you are suggesting 
would go against the bluetooth core spec, although it does help with the esp32 
issues. 
   
   The esp32 has some weird issues with connection parameters stuffing it up 
with multiple connections, this is a specific case though. I think it's better 
to handle it manually by just rejecting the request and then calling 
`ble_gap_update_params()` afterward. This may cause issues if your peripherals 
are picky about accepting their parameters though, in which case there's not 
much else to do but maintain this patch in your own code. 


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] zacwbond commented on issue #783: Make ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()

2020-04-13 Thread GitBox
zacwbond commented on issue #783: Make ble_gap_rx_l2cap_update_req() behave 
like ble_gap_rx_param_req()
URL: https://github.com/apache/mynewt-nimble/pull/783#issuecomment-612911745
 
 
   > The reson why it differs between BLE_GAP_EVENT_L2CAP_UPDATE_REQ and 
BLE_GAP_EVENT_CONN_UPDATE_REQ is basically because over L2CAP you can only 
accept or reject the requested parameters. There is no option to modify the 
parameters here.
   > Having this patch, we could end up in the situation that remote devices 
requested for parameters set A, we reply over L2CAP that we accepted those, but 
in fact we set parameters B, which is not nice.
   > If master wants to change parameters, he can always do it, but if remote 
ask for the specific parameters set, we either Reject or Accept and set it.
   > 
   > I believe, that you need to fix you application and on 
BLE_GAP_EVENT_L2CAP_UPDATE_REQ does not modify self_parameters.
   > 
   > Hope that explains.
   > If you agree please close this PR.
   
   If the peripheral requests an interval between 16 and 20, and I force the 
radio through this mechanism to use an interval of 18, it seems to me I've 
still accepted the peripheral's request because the connection interval will 
still match the peripheral's request.
   
   I think the only invalid behavior would be to accept the request but to 
choose a  parameter out of the range requested by the peripheral (or to change 
one of the parameters that isn't given as a range).  
   
   
   
   
   


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-core] apache-mynewt-bot commented on issue #2237: [WIP DNM] Added bsp and mcu packages for dialog_da14695-dk-usb

2020-04-13 Thread GitBox
apache-mynewt-bot commented on issue #2237: [WIP DNM] Added bsp and mcu 
packages for dialog_da14695-dk-usb
URL: https://github.com/apache/mynewt-core/pull/2237#issuecomment-613053754
 
 
   
   
   
   ## Style check summary
   
   ### Our coding style is 
[here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
    hw/bsp/dialog_da14695-dk-usb/src/sbrk.c
   
   
   ```diff
   @@ -23,7 +23,8 @@
static char *brk __attribute__ ((section (".data")));

void
   -_sbrkInit(char *base, char *limit) {
   +_sbrkInit(char *base, char *limit)
   +{
sbrkBase = base;
sbrkLimit = limit;
brk = base;
   ```
   
   
   
    hw/mcu/dialog/da14695/include/mcu/mcu.h
   
   
   ```diff
   @@ -41,9 +41,9 @@
"I#54=GPIO_P0,I#55=GPIO_P1"

/**
   -* \brief GPIO function
   -*
   -*/
   + * \brief GPIO function
   + *
   + */
typedef enum {
MCU_GPIO_FUNC_GPIO = 0,  /**< GPIO */
MCU_GPIO_FUNC_UART_RX = 1,   /**< GPIO as UART RX */
   @@ -115,8 +115,8 @@
#define MCU_GPIO_MODE_OUTPUT_OPEN_DRAIN 0x700/**< GPIO as an 
open-drain output */

#define MCU_GPIO_PORT0_PIN_COUNT32
   -#define MCU_GPIO_PORT0(pin) ((0 * 32) + (pin))
   -#define MCU_GPIO_PORT1(pin) ((1 * 32) + (pin))
   +#define MCU_GPIO_PORT0(pin) ((0 * 32) + (pin))
   +#define MCU_GPIO_PORT1(pin) ((1 * 32) + (pin))
#define MCU_DMA_CHAN_MAX8

#define MCU_PIN_GPADC_SEL0   MCU_GPIO_PORT1(9)
   ```
   
   


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-core] apache-mynewt-bot removed a comment on issue #2237: [WIP DNM] Added bsp and mcu packages for dialog_da14695-dk-usb

2020-04-13 Thread GitBox
apache-mynewt-bot removed a comment on issue #2237: [WIP DNM] Added bsp and mcu 
packages for dialog_da14695-dk-usb
URL: https://github.com/apache/mynewt-core/pull/2237#issuecomment-597785395
 
 
   
   
   
   ## Style check summary
   
   ### Our coding style is 
[here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
   
   
    hw/bsp/dialog_da14695-dk-usb/src/sbrk.c
   
   
   ```diff
   @@ -23,7 +23,8 @@
static char *brk __attribute__ ((section (".data")));

void
   -_sbrkInit(char *base, char *limit) {
   +_sbrkInit(char *base, char *limit)
   +{
sbrkBase = base;
sbrkLimit = limit;
brk = base;
   ```
   
   
   
    hw/mcu/dialog/da14695/include/mcu/mcu.h
   
   
   ```diff
   @@ -41,9 +41,9 @@
"I#54=GPIO_P0,I#55=GPIO_P1"

/**
   -* \brief GPIO function
   -*
   -*/
   + * \brief GPIO function
   + *
   + */
typedef enum {
MCU_GPIO_FUNC_GPIO = 0,  /**< GPIO */
MCU_GPIO_FUNC_UART_RX = 1,   /**< GPIO as UART RX */
   @@ -115,8 +115,8 @@
#define MCU_GPIO_MODE_OUTPUT_OPEN_DRAIN 0x700/**< GPIO as an 
open-drain output */

#define MCU_GPIO_PORT0_PIN_COUNT32
   -#define MCU_GPIO_PORT0(pin) ((0 * 32) + (pin))
   -#define MCU_GPIO_PORT1(pin) ((1 * 32) + (pin))
   +#define MCU_GPIO_PORT0(pin) ((0 * 32) + (pin))
   +#define MCU_GPIO_PORT1(pin) ((1 * 32) + (pin))
#define MCU_DMA_CHAN_MAX8

#define MCU_PIN_GPADC_SEL0   MCU_GPIO_PORT1(9)
   ```
   
   


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