You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/10/22 22:36:09 UTC

[GitHub] [incubator-nuttx] v01d opened a new pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

v01d opened a new pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070


   ## Summary
   
   This PR includes initial work by @btashton (seen in #1661) and work I done on top of that myself (thus this supersedes #1661).
   Brennan's initial work layed the ground work for distinguishing L2CAP (BTPROTO_L2CAP) from HCI (BTPROTO_HCI) sockets, following Linux's interface. NuttX mostly supported L2CAP sockets already but these were partially used since NuttX's upper-level bluetooth stack is actually implemented via ioctl() calls made to the socket, which internally invokes some of the intermediate layer functions directly, instead of going through network layer.
   
   To fully control a BLE device, some operations need to be performed using commands which are not related to a given connection (eg: enable advertising), and thus a HCI socket is also always needed. HCI sockets support different kind of "channels". The basic channels (supported in Linux) are the USER and RAW channels. These allow to send HCI commands directly to the device. The problem is that this implies that only one application should be operating the socket, otherwise there would be conflict. The main difference between USER and RAW channel, is that USER imposes the exclusive access, whereas RAW does not. The more modern approach supported in Linux is the MGMT (managment) channel of HCI sockets which addresses some of these problems by offering a higher-level interface (instead of via raw HCI commands).
   
   Brennan was looking to eventually support the MGMT channel. In the meantime, I was looking to support the RAW/USER channels since it is an interface used in other projects, such as the nimBLE stack (using just the "host" part), which I was interested in supporting. Thus, my work on this PR adds support for RAW channel. However, supporting the RAW channel when the host stack is supposed to use L2CAP+MGMT adds complexity and potentially allows for bad application behavior, and considering that RAW channel mostly makes sense where the whole host stack is in userspace (in contrast to the L2CAP+MGMT case, where the L2CAP part of the host layer is in-kernel), I decided to make RAW channel be supported only when NuttX's own host layer is disabled (to support an external fully userspace host-layer such as nimBLE operating via RAW HCI channel).
   
   Note you will see some TODO notes for things I didn't yet address.
   
   ## Impact
   
   Since at the moment we have not discussed the future of NuttX's BLE stack, this PR then at the moment only adds an option to disable NuttX's host layer and provide RAW HCI channel. When the host layer is enabled, this change should not affect anything.
   
   ## Testing
   
   Due to the limited ability of NuttX's own stack to be easily tested I admittedly didn't test that NuttX's own stack continues to work when it is left enabled. I would need help from someone who uses the stack to verify this.
   My testing was focused on supporting nimBLE (NuttX's stack disabled). I tested this configuration both with Brennan's recent addition of BLE support on sim platform (nimBLE then controls the PC's USB dongle) and also with the nRF52 link-layer I have developed (to be upstreamed once this is in). I have extensively tested this (specially since nimBLE operates the HCI layer quite  a bit).
   
   ## Technical Details
   
   The general idea is that the networking layer eventually reaches the HCI interface and uses bt_send() to send the HCI command to the device. To receive responses from the device, an extra callback is added to the bt netdev that is implemented in hcicore and is invoked from the reception queue. It is here that when the NuttX host-layer is disabled the callback is invoked and the packet reaches the networking layer and sends the command back to the user. If the NuttX host-layer is enabled, the command is directly handled depending on its type (an HCI response or an L2CAP packet).
   
   ## Final note
   
   Some of the complexity here is that we are still trying to support NuttX host layer. If we want to do so, it should be modified to also send packets back to the networking layer (an L2CAP packet would reach the L2CAP socket, an HCI packet would reach the MGMT channel). An alternative approach would be to embrace nimBLE harder and discard current NuttX's stack. We should discuss this soon (not here please, I mentioned this since it explains why it is designed this way right now).


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716066459


   Maybe someone would like to take a look and give some feedback? @xiaoxiang781216 @anchao @acassis ?
   Regarding testing, we should test both cases:
   1. NuttX host layer enabled
   2. NuttX host layer disabled
   
   (1) is difficult for me to try as NuttX's own bluetooth stack is quite limited. Not sure if anyone is using it actively. Maybe simply starting a bluetooth scan should be enough to excercise the host layer and see if the old ioctl() based interface still works as expected
   (2) can be tested using nimBLE using PC bluetooth dongle on sim build. I can provide a config for that and open a PR for nimBLE NuttX app. it will require to use my fork of nimBLE since that work is not yet upstream. I would like to get this PR in so that I can upstream the changes in nimBLE and finally open a PR for the nRF52 link-layer


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-714800577


   I'll leave this as draft for now since I know it needs a bit more work to be ready for merge, but I would still like for it to be reviewed and hopefully someone can test how it behaves with NuttX's stack enabled.


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716083243


   > @v01d I am fine bringing this in if you can clean up the style issues. We will keep iterating on this over multiple PRs anyway.
   
   Thanks. 
   I wanted to get some validation that this wouldn't break NuttX's bluetooth own support but I don't know what to test.
    
   > Also looks like the build system is broken right now... distclean is not cleaning up old object files.
   
   You mean in this PR? I included a modfiication to CLEAN target which is not really correct and should actually be done separately.
   


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] btashton commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716083724


   > > @v01d I am fine bringing this in if you can clean up the style issues. We will keep iterating on this over multiple PRs anyway.
   > 
   > Thanks.
   > I wanted to get some validation that this wouldn't break NuttX's bluetooth own support but I don't know what to test.
   > 
   
   I did some minimal testing, but I am less worried about that since we are moving in the right direction with both this userspace support and the new kernel interfaces.
   
   > > Also looks like the build system is broken right now... distclean is not cleaning up old object files.
   > 
   > You mean in this PR? I included a modfiication to CLEAN target which is not really correct and should actually be done separately.
   
   Yeah it's leaving object files around between builds.


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] btashton commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716069324


   @v01d I am fine bringing this in if you can clean up the style issues.  We will keep iterating on this over multiple PRs anyway.
   
   Also looks like the build system is broken right now... distclean is not cleaning up old object files.


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716208281


   I fixed styling issues and removed some unnecessary changes. This should be good to go now.


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] btashton merged pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
btashton merged pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070


   


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716084211


   > I did some minimal testing, but I am less worried about that since we are moving in the right direction with both this userspace support and the new kernel interfaces.
   
   Ok!
   
   > 
   > > > Also looks like the build system is broken right now... distclean is not cleaning up old object files.
   > > 
   > > 
   > > You mean in this PR? I included a modfiication to CLEAN target which is not really correct and should actually be done separately.
   > 
   > Yeah it's leaving object files around between builds.
   
   I just opened #2101 to deal with the change I intended to include here. Once merged I will rebase and should fix that.


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #2070: Bluetooth: support HCI/L2CAP sockets, support HCI RAW channel

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #2070:
URL: https://github.com/apache/incubator-nuttx/pull/2070#issuecomment-716242759


   Thanks!
   I just opened a PR on nimBLE repo for the changes required on that end: https://github.com/apache/mynewt-nimble/pull/878
   Once I get that merged, I will open a PR on apps repo to add support for building nimBLE as an app, which will complete the integration.


----------------------------------------------------------------
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