Tympan / Tympan_Rev_F_Hardware

Tympan Rev F Hardware Design Files
CERN Open Hardware Licence Version 2 - Permissive
4 stars 0 forks source link

Tympan Radio BLE Configuration of Tympan-Specific Service is incompatible with Capacitor Plugin for BLE #4

Open cab-creare-com opened 3 months ago

cab-creare-com commented 3 months ago

We are developing an Android app using the Angular framework built with Capacitor that will use BLE to talk to the Tympan Rev F, we can successfully write to the nRF UART Service, but we get an error when our app tries to write to the Tympan-Specific Service. For comparison, we can successfully write to the Tympan-Specific Service on the Tympan Rev E.

On the Android side, we are using the Capacitor plugin for BLE: (https://www.npmjs.com/package/@capacitor-community/bluetooth-le/v/0.5.1). When we attempt to write to the characteristic, the BLE library fails in the "write" attempt, reporting that the characteristic is not writable.

We are aware that the Tympan Remote App can successfully connect to the Tympan-Specific Service on both Rev E and Rev F, but that uses a 3-year-old Cordova-derived BLE library that may lose support in future Android versions.

I suspect the issue is related to the configuration of the BLE characteristics onboard the Tympan.

Here are the BLE services advertised by the Rev E and the Rev F: Tympan Rev E BLE Services.txt Tympan Rev F BLE Services.txt

Looking specifically at the Tympan-Specific Service ("uuid": "bc2f4cc6-..."), here is the line-by-line comparison:

WinMerge Comparison

Of note, the Tympan-Specific Service on the Tympan Rev F has three characteristics which share a UUID, which prevents them from being uniquely targeted by a "write" command.

chipaudette commented 3 months ago

Adding "write" to the already existing "notify" seems easy enough, but you'll note that that's what the second UUID entry enables (for the same UUID). So, it's possible that this is a "feature" of the underlying nRF52 library from Adafruit. We'll see. The adafruit nRF52 library is (I think) mostly a wrapper around the API provided by Nordic, so hopefully we can work around it.

Another alternative is to ask the Capacitor plugin to transmit the value via "Notify" rather than through "Write"?

cab-creare-com commented 3 months ago

I don't think it's a "feature" of the Adafruit nRF52 library, since the nRF UART Service does not have this duplicated UUID:

Here's the nRF UART Service from Tympan Rev F BLE Services.txt:

    {
        "uuid": "6e400001-b5a3-f393-e0a9-e50e24dcca9e",
        "characteristics": [
            {
                "uuid": "6e400003-b5a3-f393-e0a9-e50e24dcca9e",
                "properties": {
                    "broadcast": false,
                    "read": false,
                    "writeWithoutResponse": false,
                    "write": false,
                    "notify": true,
                    "indicate": false,
                    "authenticatedSignedWrites": false,
                    "extendedProperties": false
                },
                "descriptors": [
                    {
                        "uuid": "00002902-0000-1000-8000-00805f9b34fb"
                    },
                    {
                        "uuid": "00002901-0000-1000-8000-00805f9b34fb"
                    }
                ]
            },
            {
                "uuid": "6e400002-b5a3-f393-e0a9-e50e24dcca9e",
                "properties": {
                    "broadcast": false,
                    "read": false,
                    "writeWithoutResponse": true,
                    "write": true,
                    "notify": false,
                    "indicate": false,
                    "authenticatedSignedWrites": false,
                    "extendedProperties": false
                },
                "descriptors": [
                    {
                        "uuid": "00002901-0000-1000-8000-00805f9b34fb"
                    }
                ]
            }
        ]
    },

Note that the nRF52 UART Service has a UUID = 6e400001-..., and it has one (non-writeable) characteristic with UUID = 6e400003-... and one (writeable) characteristic with UUID = 6e400002-....

chipaudette commented 3 months ago

Unfortunately, we might be talking about two different universes. It looks like Joel might have pre-installed nRF firmware on the Rev F that I was not expecting.

Currently, he said that he installed this: https://github.com/Tympan/Tympan_Rev_F_Hardware/tree/main/TympanRadio_baseFirmware

I had thought that he was using this: https://github.com/Tympan/Tympan_Sandbox/tree/master/Tympan%20Bluetooth/nRF52840%20Firmware/nRF52840_firmware

I've asked him to confirm.

If we wanted to test ourselves, we would use one of the Bluetooth test programs that are part of the "Tympan_Library" examples. Specifically, this one: https://github.com/Tympan/Tympan_Library/tree/main/examples/02-Utility/Bluetooth/Tympan_Test_BLE

Once it's running, you can use the serial menu to send a 'v' (for 'version'). If it says "TympanBLE v0.1.0", you've got the old one. If it says "TympanBLE v0.2.4, nRF52840", you've got the new one.

UPDATE: 2024-08-27: Joel has confirmed that he is using the Apr 29 firmware at this link: https://github.com/Tympan/Tympan_Sandbox/tree/master/Tympan%20Bluetooth/nRF52840%20Firmware/nRF52840_firmware

chipaudette commented 3 months ago

To help myself reason through the changes, I'm going to do a bit of a self-review of the code. Outside opinions are welcome!

First, here's where I define my service and characteristic IDs.

// ID Strings to be used by the nRF52 firmware to enable recognition by Tympan Remote App
uint8_t serviceUUID[] = { 0xF0, 0x28, 0xE3, 0x68, 0x62, 0xD6, 0x34, 0x90, 0x51, 0x43, 0xEF, 0xAA, 0xC6, 0x4C, 0x2F, 0xBC };
uint8_t characteristicUUID[] = {  0x3C, 0xD9, 0xF7, 0x89, 0x37, 0x37, 0xAA, 0x8F, 0x71, 0x4A , 0xAD, 0x79, 0xE7, 0xE5, 0xD1, 0x06}; 

Then, I instantiate a custom BLE Characteristic using the UUID above.

BLECharacteristic myBleChar = BLECharacteristic(characteristicUUID, BLENotify | BLEWrite); //from #include <bluefruit.h>

Note that we've invoked both "Notify" and "Write" for this single instance. Looking back at @cab-creare-com's screenshot, I would assume this would result in a single entry that has both "notify" and "write" set to true. But, that's not what we see. We see two seperate entries. Why? I'm not sure.

Let's keep going and see how I handle this BLECharacterstic object. Here's where I instantiate classes to provide the four different services that we currently offer:

//Create the nRF52 BLE elements (the firmware on the nRF BLE Hardware itself)
BLEDfu          bledfu;  // Adafruit's built-in OTA DFU service
BLEDis          bledis;  // Adafruit's built-in device information service
BLEUart_Tympan  bleService_tympanUART;    //Tympan extension of the Adafruit UART service that allows us to change the Service and Characteristic UUIDs
BLEUart         bleService_adafruitUART;  //Adafruit's built-in UART service

Next, here's where I invoke the services:

  // Configure the standard Adafruit UART service
  bleService_adafruitUART.begin();

  // Configure and Start our custom tympan-specific BLE Uart Service
  bleService_tympanUART.setUuid(serviceUUID);
  bleService_tympanUART.setCharacteristicUuid(myBleChar);
  bleService_tympanUART.begin();

  // Start our custom service
  myBleChar.setProperties(CHR_PROPS_NOTIFY);  //is this needed?
  myBleChar.setPermission(SECMODE_OPEN, SECMODE_NO_ACCESS); //is this needed?
  myBleChar.begin();

The code above is mostly copy-paste, so I don't know what CHR_PROPS_NOTIFY, SECMODE_OPEN, and SECMODE_NO_ACCESS do. Maybe these three constants hold the source of the problem? I'm guessing that these items are defined in the Adafruit or nRF APIs, so I ought to find them fairly readily. Or, the problem could be in my extension of their BLEUart class when I created my BLEUart_Tympan.

Finally, just for completeness, here's where I set up the BLE advertising. Note that I have to attach our custom service to it. If our custom service isn't in the BLE advertising packet, the Tympan_Remote App doesn't recognize it as a Tympan.

 // Advertising packet
  Bluefruit.Advertising.addFlags(BLE_GAP_ADV_FLAGS_LE_ONLY_GENERAL_DISC_MODE);
  Bluefruit.Advertising.addTxPower();
  // Include bleuart 128-bit uuid
  Bluefruit.Advertising.addService(bleService_tympanUART);   //this one is necessary for the TympanRemote App to see this device
chipaudette commented 3 months ago

Looking at the BLEUart.cpp from the Adafruit nRF library, I see that lin lines 51-65 that they purposely use different UUID values for TX and for RX. This correctly reflects the screenshot by @cab-creare-com, so it makes sense. That's good.

image

You'll note that these two UUID values are hard-coded. I cannot alter them through class method or anything. And, unfortunately, these are not the UUID values expected by the Tympan_Remote app. So, this class can't be used as-is. This is why I chose to extend the class to become "nRF52_BLEUart_Tympan.h". I extended it to allow me to se my own UUID values.

In my nRF52_BLEUart_Tympan.h, we see that my code purposely sets both the TX and RX to the same UUID. So, it makes sense that they're both showing up with the same UUID. That's good, too.

image

Presumably, I'm using the same UUID because that's what the old Bluetooth unit did (the BC127 from the Tympan Rev D and Rev E). The screenshot from @cab-creare-com confirms that this is what the old unit produces. It's good to see this confirmation.

It appears that the problem is that the current code produces two entries in @cab-creare-com's screenshot. The BC127 produces only produces a single entry. Why are they different? Presumably, if we can make the Tympan/Adafruit firmware show up as a single entry, it'll be the victory that we need?

chipaudette commented 3 months ago

Based on the two posts above, I've found the first revision to try.

First, simply try commented out the line below from nRF52_BLE_Stuff.h

Comment Out: myBleChar.setProperties(CHR_PROPS_NOTIFY); //is this needed?

In the constructor for myBleChar, I was setting the properties to WRITE+NOTIFY. In the line of code above, I'm setting the properties to just NOTIFY. This could be the problem.

Additionally, the the setPermissions() needs to be changed. Per the nRF library's BLEService.cpp, the two arguments are read permission and then write permission. So, presumably, we need both to be true. So, the line should look like:

Edit To Be: myBleChar.setPermission(SECMODE_OPEN, SECMODE_OPEN);

My main fear with this approach is that, by me inheriting from BLEUart, I'm also inheriting its behavior of trying to set up two separate characteristics.

image

If we really want to squeeze into one characteristic (because that's what the Tympan_Remote expects?), it seems likely that we'll have to override this inherited behavior. We'll have to re-write it to simply invoke a single characteristic, thereby maintaining both its read and write (and notify) qualities.

Re-writing more of Adafruit's BLEUart class makes me nervous because it introduces the opportunity for more bugs. If simply changing the two lines above fails to work, however, it'll have to be tried.

cab-creare-com commented 3 months ago

Better late than never: Yes, my factory Rev F is running BLE module firmware: TympanBLE v0.2.4, nRF52840.

Tympan_Test_BLE: Starting setup()...
Setup complete.
SerialManager Help: Available Commands:
 h: Print this help
 k/K: AUDIO: Incr/Decrease Digital Gain
 c/C: SYSTEM: Enable/Disable printing of CPU and Memory usage
 v:   BLE: Get 'Version of firmware from module
 n:   BLE: Get 'Name' from module
 t:   BLE: Get 'Advertising' statu
 g  : BLE: Get 'Connected' status
 J:   Send JSON for the GUI for the Tympan Remote App

serialManager: BLE module firmware: TympanBLE v0.2.4, nRF52840
chipaudette commented 3 months ago

After digging a bit deeper, it looks like I only need to override BLEUart::begin() and not anything else in that class. That feels like a modest extension and, therefore, relatively safe.

So, I've made the changes and pushed them to a new branch of the Tympan_Sandbox repo: https://github.com/Tympan/Tympan_Sandbox/tree/feature_BLE_combinedRXTX/Tympan%20Bluetooth/nRF52840%20Firmware/nRF52840_firmware. It compiles but it is untested (I don't have a Rev F here, nor the programming nest, nor the J-link interface).

@cab-creare-com, How fast do you want to move forward with this? If you can wait until later in the week (Friday?), I can probably come in and flash your Tympan's nRF chip. Or, we can try to set you up to do it yourself. If you want to do it yourself, let me know.

cab-creare-com commented 3 months ago

@chipaudette It can wait until Friday.

chipaudette commented 3 months ago

I tried the updated firmware. I think I've had success!

Below are three screenshots from the nRF Connect App. The one on the left is a Rev F where the nRF52840 is running the baseline firmware (Apr). In the middle is a Rev E, which is the old BC127 module. On the right is the Rev F where the nRF52840 is running my new candidate firmware.

image

As can be seen, the Apr 29 firmware generates three entries with the UUID ending "93c". The BC127 only produced one entry for the UID ending "93c". Now, with the new 8/28 firmware, our nRF52840 only produces one entry, as well.

This new 8/28 firmware does work with the TympanRemote App (on Android). Hopefully, it'll work with the Capacitor Plugin, too!

cab-creare-com commented 2 months ago

Success! Using the 8/28 firmware, we're able to use the Capacitor Plugin for BLE to communicate with the Tympan-Specific Service on both Tympan Rev E and Tympan Rev F.

Thanks!

chipaudette commented 2 months ago

Excellent! I've merged the branch into master.

@biomurph, should we move the firmware over to the Rev F Hardware repo to overwrite the out-of-date firmware that lives there?

biomurph commented 2 months ago

Yes!