start_advertising interval does not accept the minimum 20ms value and may not range check correctly #2930
Description
I was reading https://developer.apple.com/accessories/Accessory-Design-Guidelines.pdf and page 126, section 35.5 recommends "an initial 30s of 20ms intervals" for advertising if I'm understanding this correctly.
I tried it on CircuitPython and it didn't behave. I've just run it again against 5.3.0 with 20200519 libs.
Adafruit CircuitPython 5.3.0 on 2020-04-29; Adafruit Circuit Playground Bluefruit with nRF52840
>>>
>>> from adafruit_ble import BLERadio
>>> from adafruit_ble.advertising.adafruit import AdafruitColor
>>> ca = AdafruitColor()
>>> ca.color = 0xee0000
>>> ca
Advertisement(data=b"\x0a\xff\x22\x08\x06\x00\x00\x00\x00\xee\x00")
>>> ble = BLERadio ()
>>> ble.start_advertising(ca, interval=0.020)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "adafruit_ble/__init__.py", line 185, in start_advertising
_bleio.BluetoothError: Unknown soft device error: 0007
I had a brief discussion with @jepler on discord about this and we both assumed it was an unfortunate FP-ism. I had a glance at the code and it looks like it might unfortunately reject via an informational exception a 0.020 value because of the 30-bit fp conversion and then comparison of < 0.020f. The nice exception doesn't happen, I tried a few values to work out what's going on.
>>> ble.start_advertising(ca, interval=0.019)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "adafruit_ble/__init__.py", line 185, in start_advertising
_bleio.BluetoothError: Unknown soft device error: 0007
>>> ble.start_advertising(ca, interval=0.001)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "adafruit_ble/__init__.py", line 185, in start_advertising
ValueError: interval must be in range 0.0020-10.24
Are these pair (https://github.com/adafruit/circuitpython/blob/master/shared-bindings/_bleio/Adapter.c#L36-L37) supposed to read 0.020 (20ms) rather than 0.0020 (2ms)?
The code is also not doing an explicit conversion to integer so gets truncation (round towards zero) semantics and judging by a test of this code that will also punt a 30bit 0.020 value into out of range territory:
#define SEC_TO_UNITS(TIME, RESOLUTION) (((TIME) * 1000000) / (RESOLUTION))
#define UNIT_0_625_MS (625)
int main(int argc, char *argv[]) {
float f1 = 0.020f;
/* 0.020 into Schmidt FP tool, zero lower two digitals, take "actual" value - this is way beyond 32bit prec. */
float f2 = 0.01999999582767486572265625f;
float f3 = 0.02001;
int i1, i2, i3;
if (f2 < f1) {
printf("TOO SMALL\n");
};
i1 = SEC_TO_UNITS(f1, UNIT_0_625_MS);
i2 = SEC_TO_UNITS(f2, UNIT_0_625_MS);
i3 = SEC_TO_UNITS(f3, UNIT_0_625_MS);
fprintf(stdout, "%f is %d and %f2 is %d and %f2 is %d\n", f1, i1, f2, i2, f3, i3);
}
Output is:
TOO SMALL
0.020000 is 32 and 0.0200002 is 31 and 0.0200102 is 32
The middle one is actually the 0.020 with MicroPython's 30bit conversion applied and comes out as 31 which is below minimum.
For anyone else stumbling across this issue, the value below works as a workaround that can be left in place after any fixes:
>>> ble.start_advertising(ca, interval=0.02001)