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/25 15:33:33 UTC
[GitHub] [mynewt-nimble] zacwbond opened a new pull request #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
zacwbond opened a new pull request #783: Make ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
URL: https://github.com/apache/mynewt-nimble/pull/783
`BLE_GAP_EVENT_L2CAP_UPDATE_REQ` and `BLE_GAP_EVENT_CONN_UPDATE_REQ` share the same `conn_update_req` event structure when they are sent to the application callback, but they do not behave the same way.
`ble_gap_rx_param_req()` creates a self_params, allowing the application to adjust the parameters to something acceptable. `ble_gap_rx_l2cap_update_req()` just leaves the `self_params` structure NULL and otherwise ignores it. Now they both behave in the same way.
In my specific case, the ESP32 platform I am using crashes if I don't enforce a specific set of parameters when multiple peripherals connect. Since the peripherals use the L2CAP method to make the request, this change eliminates the crash.
----------------------------------------------------------------
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] zacwbond commented on issue #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
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-614686578
> @zacwbond Could you please add additional check as @andrzej-kaczmarek suggested?
Got it, thanks. I'll make the change as soon as I am able.
----------------------------------------------------------------
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 #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot commented on issue #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot 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-603910393
<!-- 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 issue #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
rymanluk 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-613907117
@andrzej-kaczmarek I agree.
----------------------------------------------------------------
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 issue #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
rymanluk 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-613311714
@zacwbond Actually on the second thought, why not to allow application to fine tune that parameters. It is up to application to behave correctly here.
If there will be no voices against that I will merge it. Thanks
----------------------------------------------------------------
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 #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
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-610571660
I’ve been seeing this happen as well on esp32. Only way to prevent it I have found is to reject the peer request then manually change the parameters after. This PR makes more sense.
----------------------------------------------------------------
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 #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot 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-604091459
<!-- 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] zacwbond commented on issue #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-nimble] apache-mynewt-bot removed a comment on issue #783:
Make ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment 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-603910393
<!-- 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] andrzej-kaczmarek commented on issue #783: Make
ble_gap_rx_l2cap_update_req() behave like ble_gap_rx_param_req()
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek 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-613356581
if we are going to allow adjusting min/max interval by application, we also need to make sure that selected range is within original range, otherwise we violate spec as @rymanluk said before. adding this extra check would both resolve original issue and would not make our host accept parameters that violate spec.
----------------------------------------------------------------
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