Implemented the ability to set the transmitter power on nRF52 devices

[leonwilly]

I added the ability to set the transmitter power on the nrf52 devices. I also modified the Adafruit_CircuitPython_BLE python file and will commit those changes as well. Other devices will need to have the hal interface added or a dummy interface created if not possible. Alternatively a compile time flag could disable this for all but this particular device.

[leonwilly]

This is just an initial pull request to discuss the additional device implementation details and or compiler flag. It will fail build test for other devices. Also I elected not to preform transmitter value checking. It can be added at the expense of code size.

[tannewt]

Thanks for the PR! One question. The code looks good otherwise. Thanks!

[tannewt]

You shouldn't need an extra flag. _bleio should be enough granularity.

[tannewt]

You'll also need to edit ports/nrf/common-hal/_bleio/Adapter.h to add a tx_power field. That is why the CI is failing.

[leonwilly]

You'll also need to edit ports/nrf/common-hal/_bleio/Adapter.h to add a tx_power field. That is why the CI is failing.

Damn I forgot to commit that file! Good catch that explains some failures.

[leonwilly]

You shouldn't need an extra flag. _bleio should be enough granularity.

Because the shared-bindings files Adapter.h and Adapter.c have been modified along with the __init__.py in the Adafruit_CircuitPython_BLE library (Which I haven't submitted a pull request for that yet because I didn't have time on my lunch break). Won't the other devices need to have a dummy implementation of the get_tx / set_tx functions in common-hal? If so I will take a look at the other devices and implement the hal functions when I get time.

[kevinjwalters]

@dhalbert mentioned in #2818 that there's separate control over advertising tx power and packets for connections. Are both of those values or some global value being set by this change?

[leonwilly]

@dhalbert mentioned in #2818 that there's separate control over advertising tx power and packets for connections. Are both of those values or some global value being set by this change?

@dhalbert Is correct according to Nordic API reference there are 3 types of power roles.

Enumerator
--
BLE_GAP_TX_POWER_ROLE_ADV | Advertiser role.
BLE_GAP_TX_POWER_ROLE_SCAN_INIT | Scanner and initiator role.
BLE_GAP_TX_POWER_ROLE_CONN | Connection role.

My implementation strictly sets the Advertiser role. Maybe this property should be renamed advertising_tx_power?

[leonwilly]

Maybe instead of a property this should simply be an argument added to the start_advertising method?

[dhalbert]

Maybe instead of a property this should simply be an argument added to the start_advertising method?

I think this may be a better solution. The original .tx_power was added when I was not as aware of the many variant ways of controlling tx power. Another place to look is what the bleak-related and Bluetooth HCI interface provide, so we can make this work across other platforms. I think it may not actually be possible to change the power in the ESP32 HCI firmware (I cannot remember at the moment and am away from be able to look this up).

[leonwilly]

Maybe instead of a property this should simply be an argument added to the start_advertising method?

I think this may be a better solution. The original .tx_power was added when I was not as aware of the many variant ways of controlling tx power. Another place to look is what the bleak-related and Bluetooth HCI interface provide, so we can make this work across other platforms. I think it may not actually be possible to change the power in the ESP32 HCI firmware (I cannot remember at the moment and am away from be able to look this up).

Functions
esp_err_tesp_ble_tx_power_set(esp_ble_power_type_tpower_type, esp_power_level_tpower_level)
Set BLE TX power Connection Tx power should only be set after connection created.

Return
ESP_OK - success, other - failed

Parameters
power_type: : The type of which tx power, could set Advertising/Connection/Default and etc

power_level: Power level(index) corresponding to absolute value(dbm)

It looks like its supported at first glance of the Espressif documentation.

[dhalbert]

I think this may be a better solution. The original .tx_power was added when I was not as aware of the many variant ways of controlling tx power. Another place to look is what the bleak-related and Bluetooth HCI interface provide, so we can make this work across other platforms. I think it may not actually be possible to change the power in the ESP32 HCI firmware (I cannot remember at the moment and am away from be able to look this up).

Functions
esp_err_tesp_ble_tx_power_set(esp_ble_power_type_tpower_type, esp_power_level_tpower_level)
Set BLE TX power Connection Tx power should only be set after connection created.

Return
ESP_OK - success, other - failed

Parameters
power_type: : The type of which tx power, could set Advertising/Connection/Default and etc

power_level: Power level(index) corresponding to absolute value(dbm)

It looks like its supported at first glance of the Espressif documentation.

I was thinking of the HCI protocol implementation, not the ESP-IDF. Looking through the HCI commands, I see ways of reading the power used, but setting the power for BLE can only be done for advertising, with "LE Set Extended Advertising Parameters", and Extended Advertising is not turned on in the NINA-FW we are using.

[tannewt]

I'm picking up this PR as a way to get back into nRF work.

[tannewt]

@dhalbert Please do a final review on this. I tested it on a Clue and read the RSSI changes with nRF Connect on my phone.

[dhalbert]

Looks good! Thanks!

updated

[kevinjwalters]

Did this go into 6.3.0?

[dhalbert]

Did this go into 6.3.0?

No, what's in 6.3.0 is in the release notes: https://github.com/adafruit/circuitpython/releases/tag/6.3.0. In general 6.3.0 added boards and a limited set of important fixes, not feature additions.

[kevinjwalters]

I asked because there's some strangeness mentioned in Adafruit Forums: Bluefruit advertisement.complete_name not showing