virtualhub: Implement BTStack bluetooth. #435
Conversation
jaguilar
force-pushed
the
virtualhub-bluetooth
branch
2 times, most recently
from
bb537a4 to
9aea25d
Compare
|
Okay, so this does work on a system with libusb and pkg-config installed. However, I see now that the test host does not have these libraries. We can either vendor in libusb, or we can install it in the test runner. What is the preferred solution? |
|
Nice! I hope to try this out this week. Re: CI: Normally installing is preferred but in this case we don't need either. The virtual hub had both a mock usb and mock Bluetooth driver, with only the latter enabled by default. Since the Bluetooth driver will now be a real driver and the CI won't need to talk to real hardware, we can have the virtual hub use the USB mock driver to simulate stdout instead. For local testing we can have both enabled. The mock USB driver will still provide stdout to the host terminal which is nice as a sanity check, and the Bluetooth driver will also provide it if something is connected. |
|
It's probably just me, but I wouldn't ordinarily run a script downloading things with regexes somewhere in my home directory with a nonzero history of bricking something 😄 I suppose we could have a folder The BTstack posix example has a way of specifying which device to use. I expect most test cases to use two dongles: one for the virtual hub, and one for the OS to test e.g. rfcomm or Pybricks Code. Could we do that with an environment variable? How does it currently determine which one to use? |
Ahaha, but in my case it bricked my dongle because I didn't just use the firmwares in this directory, I renamed one of them. Ultimately the risky part of the thing is that the firmwares are used by name and the btstack chipsets don't seem to have a good way to validate that a firmware file is correct for a module before sending it down. And/or that there is a module that will accept a firmware that will brick it. That risk remains whether the files are in the home directory or elsewhere.
I can checkout to a particular git sha and/or tag in my download script. That would be fine. On the other hand, every time a new chipset comes out it would be a mysterious bug for the first person who tries to use that chipset with our test bench. Up to you.
|
jaguilar
force-pushed
the
virtualhub-bluetooth
branch
2 times, most recently
from
a807400 to
0f67ca8
Compare
collect_bt_patches.py collects bluetooth module firmware patch files for the most popular Broadcom and Realtek modules into a directory where they can be used by the virtualhub for initializing bluetooth modules.
- Changes pbdrv_bluetooth_btstack_set_chipset to convey all necessary information to set the correct chipset both from the read local version information command as well as events from the USB subsystem. - Adds a POSIX implementation for pbdrv_bluetooth_btstack_set_chipset. This supports the most common Realtek and Broadcom chipsets, which comprise the vast majority of USB dongles. - Sets up the virtualhub platform to use this chipset. - Adjusts the runloop to check for readability and writability of file descriptors, which is required for the libusb transport.
- Implements HCI logging in bluetooth BTStack. - Adds a stderr version of uart_debug_first_port. - (Revert?) Enables debug logging for virtualhub bluetooth.
jaguilar
force-pushed
the
virtualhub-bluetooth
branch
from
0f67ca8 to
1bca902
Compare
|
Added the ability to manually specify the device using To test: So the wireless device is on bus 1 port 3. btstack ignores the bus number, so we'll just be looking at the port: The correct device is used. Initialization fails and the program hangs forever.
This is now implemented.
Need some more practical guidance on this. Right now virtualhub does not build on CI due to the libusb dependency. Practically speaking, do I need to create a new target that has the dep and not have that target on CI? Or make the dep on libusb weak instead? |
|
Thanks for the updates! I think we already have a candidate bug where this PR would be a big help: pybricks/support#2497
I'll have a look at this when I get to around to the review. My thinking was that we would not be building the Bluetooth driver at all on the CI build, just like the I suppose it would help if I switched the current virtualhub to use the virtual usb driver by default, so we only enable the bluetooth simulation driver for local use. I'll try and do that today. |
FYI, there is a |
|
FYI, BTStack merged my pull request, which means at least in the development branch there is also the possibility to specify the USB bus to further disambiguate which device you're referring to. I'm not sure if we would want to cherry pick that into our view of btstack (and I don't know how to do that), or if we would just wait until it makes it to mainline. |
Updated the master branch to use the USB mock on the CI so we have flexibility for Bluetooth stuff locally. Sorry for the delay. This will still print Pybricks stdout to native stdout in addition to whatever we will do with real Bluetooth, so this is still useful for non-bluetooth developments. |
|
Two TP Link UB500 Bluetooth dongles on the way! |
|
I still think it would be better/simpler if we just stick with HCI UART for the virtual hub. In Bleak we recently got a contribution to do automated testing using the Bumble Bluetooth stack. This is a Bluetooth stack written entirely in Python. It can create a pty to simulate an HCI UART controller that can be controlled entirely from Python. We could use this to write CI tests to test all of our Bluetooth code (well, at least BTStack). And for manual testing, it can easily be switched out for read hardware. |
|
@dlech I’m not opposed to also supporting bumble. I think it would just involve adjusting the transport to connect to a pty instead of using libusb. I don’t think that anything in this pr precludes doing that, and I suspect it may be useful to have both options (virtual hub supports physical hardware and simulated controller). |
|
When initially proposing this in pybricks/support#2461, I was thinking to use it more as a development tool, for use with real hardware, given the lack of a real debugger on the LEGO hubs:
I don't really mind exactly how we implement it. Using it with a dedicated USB dongle as proposed here is fine by me. If we can do additional automated CI testing with a variation of this then that's a nice bonus but not a hard requirement for me personally. |
| uint16_t manufacturer; | ||
| uint16_t lmp_pal_subversion; | ||
|
|
||
| } pbdrv_bluetooth_btstack_device_discriminator; |
There was a problem hiding this comment.
| } pbdrv_bluetooth_btstack_device_discriminator; | |
| } pbdrv_bluetooth_btstack_device_discriminator_t; |
| """ | ||
|
|
||
| import argparse | ||
| import os |
| from pathlib import Path | ||
|
|
||
| # Destination directory in user's cache directory | ||
| DEST_DIR = Path.cwd() / ".bt_firmware" |
There was a problem hiding this comment.
I suppose this assumes we must run from the workspace base directory? Should we raise an error if that isn't the case?
There was a problem hiding this comment.
Since this is bluetooth firmware, should we perhaps put it in lib/pbio/drv/bluetooth/firmware/libusb?
|
Hmm, I thought I'd try to do the review in the fancy editor. Turns out all feedback was lost 😞 . |
| // Parses a USB path in PYBRICKS_VIRTUALHUB_USB_PATH and sets | ||
| // hci_transport_usb_set_path using the result. The expected format is | ||
| // 8 bit unsigned integers separated by slashes. If an incorrect format | ||
| // is passed, no path is set. |
There was a problem hiding this comment.
It would be nice to add a few words here as to what it will do if no path is provided. It looks like ultimately it will try to find the first device that matches is_known_bt_device. We could add a printf in this module to display what we'll choose.
| ++usb_path; | ||
| --len; | ||
| } else { | ||
| pbdrv_uart_debug_printf("Invalid USB path format (should be <port>[/<port>...])\n"); |
There was a problem hiding this comment.
What does this do on POSIX? We can use regular printf (or, introduce pbio/debug, mapping to printf on POSIX -- in the long run).
There was a problem hiding this comment.
I have a patch that converts this to fprintf(stderr, ...) in my local tree. For now it does nothing in the commits I've sent out so far.
| do_poll_handler = false; | ||
| btstack_run_loop_base_poll_data_sources(); | ||
|
|
||
| #if PBDRV_CONFIG_BLUETOOTH_BTSTACK_POSIX |
There was a problem hiding this comment.
I think we could have a pbdrv_bluetooth_btstack_poll_hook() that does the following. On the other implementations, this can be a no-op.
|
I think I'm almost getting this running (I see HCI traffic) but it faults at: static bool le_device_db_tlv_fetch(int index, le_device_db_entry_t * entry){
btstack_assert(le_device_db_tlv_btstack_tlv_impl != NULL); <---This would be set by You were able to run it without, so maybe I'm missing a step somewhere? Update 1: Looks like I can get around it by disabling the Update 2: Can't seem to get Bluetooth operations going just yet. What did you test it with so far? |
|
With a bit more hacking, I think it is sending the right HCI commands to e.g. start advertising but I don't seem to get any actual radio output. 🤔 |
|
OK, it does appear that the very first attempt after I think I'll have to go back now and see which patches I tried were really needed, if any, and build from there. |
|
That said, just this testing has uncovered two unrelated bugs because we can debug it so easily now. Thanks @jaguilar! |
|
Hmm, this doesn't seem to actually uploads any firmware files?
So it seems somewhat implausible that this was uploading firmware to the UB500, but maybe I'm missing something? Instead, I was able to get something going using the stock firmware loaded by the OS. I wonder if you have been using the stock firmware too? The |
|
I think we could be a bit pragmatic here. This addition is very useful, but it will only be used by a handful of developers. And the UB500 dongle recommended above is only $10. If someone comes along and really wants to use another dongle, we can just add support for it then. In its current form, it seems hard to guarantee that anything untested would otherwise work. If we take this approach, we can use the PID and VID build flags that BTstack already has so it never accidentally picks up anything else. We can specify the port number in case you want to use two for certain test scenarios. (EDIT: Can't pick port in this case.) There is also just one firmware and config file to use. I wonder if we even need to upload a firmware at all? We could include it in If the device is not detected, the virtualhub will work normally without Bluetooth support to just keep things simple. This also lets you dedicate the UB500 for virtual hub development (setting up Does that make sense? |
|
Whichever way we go, I've made made some generalizations in #442 which should help here. Once that is merged, we can rebase this one to make use of the changes. Anything else that touches existing platforms can be added to #442 for independent testing. That's kind of important because the following diff here breaks EV3 and SPIKE 😄. -uint16_t lmp_pal_subversion = pbio_get_uint16_le(&rp[7]);
+device_info.lmp_pal_subversion = pbio_get_uint16_le(&rp[6]); |
|
Thanks for taking the time to look at this. I had a chance to go back and check my original stack of commits today. Here's what I'm seeing on my end.
|
|
Ah, so that makes sense. The good news is that we are able to get firmware going on it. This actually solves my problem of having to otherwise power cycle it, but maybe there's a way to get it in a good state even without uploading any firmware. We can get past all of the missing stuff by just doing: btstack_chipset_realtek_set_firmware_file_path(".bt_firmware/rtl8761bu_fw.bin");
btstack_chipset_realtek_set_config_file_path(".bt_firmware/rtl8761bu_config.bin");This makes everything reasonably simple, especially if we did choose to go ahead with just this one device. Unfortunately enabling the But I'm leaning towards doing a quick device check early on in our own new init hook before BTstack even starts. If all checks pass, then we know BTstack can take it from there. If not, we can gracefully say that the device is not there and launch the Virtual Hub normally without trying to somehow escape from the runloop later. |
4 participants
These commits allow executing real bluetooth code using virtualhub combined with an appropriate dongle. This has been tested with TP Link UB500, however, it should work with any USB dongle for which an appropriate init script exists.
Talking of init scripts, this pull request also contains a tool that collects the most common bluetooth firmwares into
~/.cache/pybricks/virtualhub/bt_firmware. Given the dongle referenced above, and after running the provided tool (pybricks-micropython/tools/collect_bt_firmware.py),bricks/virtualhub/build-debug/firmware.elf tests/virtualhub/basics/hello.pywill emit logs like the following:Tail of virtualhub.elf log
The best way to find a supported dongle is:
./tools/collect_bt_firmware.pyThis has been tested on Windows with WSL and usb-ipd-win.