Add immediate parameter to BLE connection commands.

[h2zero]

Description:

When sending a command to read/write a characteristic on a BLE server this adds a new parameter to cause the action to be
performed immediately. This will stop the scan in progress to perform the actions queued.

Example use:

mosquitto_pub -t home/OpenMQTTGateway/commands/MQTTtoBT/config -m '{
  "ble_write_address":"AA:BB:CC:DD:EE:FF",
  "ble_write_service":"cba20d00-224d-11e6-9fb8-0002a5d5c51b",
  "ble_write_char":"cba20002-224d-11e6-9fb8-0002a5d5c51b",
  "ble_write_value":"TEST",
  "value_type":"STRING",
  "ttl":4,
  "immediate":true }'

Checklist:

• The pull request is done against the latest development branch
• Only one feature/fix was added per PR and the code change compiles without warnings
• I accept the DCO.

[DigiH]

Just got back and did some brief testing with this and found some puzzling results so far:

• Both READ and WRITE do not work, if the relevant device MAC ID is included in the white-list. Removing a MAC ID from the white-list makes all my READ tests work fine and immediate for me - only been testing with my Playbulb Candles reading the battery status so far and this doesn't seem to be an immediate effect of this change but a general 0.9.9 beta or even previous issue.
• WRITE so far only gives me "success":false results, even with commands which previously sporadically worked. So the WRITE connection seems to be immediate, but on top of the constant "success":false results many times the response also comes back with nonsensical "write":"AC:E6:4B:XX:XX:XX" where the MAC ID is of a previous tested device, or something like "write":"T/DDF44BXXXXXX", where in all cases all I sent was a simple "ble_write_value":"000000ff","value_type":"HEX" colour command.

Hope this helps and would be good to know what you or others see with different devices.

P.S.: I think the nonsensical "write" responses mentioned above might have been caused by the ever annoying straight vs. curly double quotes when copying and pasting the various test commands ;)

BUT, the constant "success":false results remain for me with my Playbulb candles. I just switched back to OMG 0.9.8 vs. the current where 0.9.9 beta + this change and my frist try of

{"ble_write_address":"3D:B5:4B:XX:XX:XX","ble_write_service":"ff02","ble_write_char":"fffc","ble_write_value":"000000ff","value_type":"HEX","ttl": 10}

resulted in a success with

{"id":"3D:B5:4B:XX:XX:XX","service":"0xff02","characteristic":"0xfffc","write":"000000ff","success":true}

albeit with a long delay as expected.

0.9.9 beta with #1136 and this always produces "success":false

Any of the other 13 commits possibly relevant to test this fully?!? …

[DigiH]

No :( latest development branch + this change results in constant "success":false results for me, with or without "immediate":true

Also the occasional nonsensical "write" responses are back, even after double checking all the double quotes.

{"id":"3D:B5:4B:XX:XX:XX","service":"0xff02","characteristic":"0xfffc","write":"T/3DB54BXXXXXX","success":false}

[DigiH]

BUT 0.9.8 with this change works flawlessly for my Playbulb Candles ;) - hmmm

P.S.: OK, so taking #1136 out of the development branch, eveything is fine for me with the latest dev build as well. Constant imediate writes.

[h2zero]

Thanks, looks like there is some weird things going on with write responses with your devices. Might need to revert that commit and add it as a command option instead.

[DigiH]

@h2zero Any idea why I'm still seeing these weird responses? Even now every now and then I get things like

{"id":"3D:B5:4B:XX:XX:XX","service":"0xff02","characteristic":"0xfffc","write":"D:B5:4B:XX:XX:XX","serv","success":false}

and the examples above.

Could well also be with my devices, but it seems to be a 0.9.9/dev only issue, as I didn't come across it in 0.9.8.

[h2zero]

I'm not sure what is causing this at the moment. I'll look into it.

[h2zero]

@DigiH could you post the command you send when you see these incorrect responses?
I'm not completely sure but the command you posted above:

{"ble_write_address":"3D:B5:4B:XX:XX:XX","ble_write_service":"ff02","ble_write_char":"fffc","ble_write_value":"000000ff","value_type":"HEX","ttl": 10}

This looks problematic in the way you have "000000ff" in quotes. That said I think the value handling also needs some work.

Edit: Sorry, don't worry about the comment about the quotes, I forgot how this code worked internally and that should be fine.

[h2zero]

@DigiH, I've updated the value handling hopefully that resolves the data errors.

[DigiH]

@DigiH, I've updated the value handling hopefully that resolves the data errors.

@h2zero thanks for looking into this, but with the change the occasional strange error responses still persist, now always only with the following two variations:

{"id":"3D:B5:4B:XX:YY:ZZ","service":"0xff02","characteristic":"0xfffc","write":"YYZZ","success":true}
{"id":"3D:B5:4B:XX:YY:ZZ","service":"0xff02","characteristic":"0xfffc","write":"D:B5:4B:XX:YY:ZZ\",\"serv","success":true}

where especially the extra ,"serv"," puzzles me,

but the majority of responses, which previously worked fine, now always come back with

{"id":"3D:B5:4B:XX:YY:ZZ","service":"0xff02","characteristic":"0xfffc","write":"","success":true}

with the write field empty, and as such failing in turning the device on with the desired colour. So for me this latest change actually broke it.

Similalrly to #1136 always producing "success":false for these devices.

You mentioned above that it might be a case of strange write behaviour of these Playbulb devices, and as I'm currently controlling all the WRITE commands through a Raspberry Pi with

https://github.com/Heckie75/Mipow-Playbulb-BTL201

there is no immediate need to switch over, it just would have been nice to also control these lights with OMG, with the better deployability for BT reception than with a single Raspberry.

Maybe a release and wider spread testing of the immediate WRITE functionality with more device types is needed to see if there are some code adjustments necessary, or if there just will be awkward devices like mine.

The READ command however, which I have switched over to OMG since 0.9.7, works flawlessly to get the battery charge status of the candles, and the immediate parameter is a welcome addition to it :)

[h2zero]

Thanks for the details @DigiH, I'm going to build a device and see if I can simulate the error. It's odd that this patch would have changed anything in this regard but it may be exposing an issue that's gone unnoticed until now.

I should have a solution later today if I can reproduce it.

[DigiH]

Thanks a lot @h2zero - I also wanted to ask you if the previously mentioned white-list problem is reproducible for you.

With the latest dev branch, minus #1136, minus the latest change here it also doesn't seem to be enough to send a new white-list without the MAC address, the WRITE commands don't work and don't produce any responses at all, until I restart the ESP32 and apply a new white-list which cannot contain any devices which I want to write to.

[h2zero]

I don't have any thoughts on that right now but I'll have a look at this as well.

[DigiH]

@h2zero just an update testing with the latest changes

• No more strange nonsensical error responses as described above with my devices
• The immediate parameter is now a lot more immediate than before for me, where it could take around 3-4 seconds before it is now always < 1 second or virtually instant.

Double thumbs up and a big thanks from me :)

The issue with the white-list remains though. As soon as I add a test device MAC to the white-list all consecutive WRITE commands are ignored, i.e. there is nothing happening on the device, nor are there any response messages. And still, sending a new white-list without the affected device MAC doesn't remedy the situation. Only a restart of OMG brings back writing capabilities to the device.

This white-list problem should probably be logged and be addressed in a separate issue, as this immediate parameter PR here looks perfectly implemented to me now.

P.S.: An update regarding the white-list issue. Have been testing more and found the following, unfortunately related to READ/WRITE commands

• After issuing a READ or WRITE command, with the device ID/MAC address being in a white-list, the command is not being executed, BUT ALSO the device is not being included in the subsequent scans any longer and not being reported, even when it has been reported regularly as being a white-list device before any READ or WRITE commands have been issued.

[h2zero]

@DigiH Thanks! I'm still not sure why the white/black list is an issue with this but I suspect it has to do with the flags being updated. I'll look into this today.

[DigiH]

@h2zero Thanks, I also just went back to 0.9.8, which I have been using to reliably READ the battery level states of all my Candle devices, albeit with a delay, and not having any of them in the white-list.

Adding the devices to the white-list in 0.9.8 doesn't exclude them from subsequent scans as currently in the dev branch, but it also does make the READ commands very unreliable, only succeeding very occasionally after they have been added to the white-list. So this doesn't seem to be a new issue, just showing up more predominantly with the above described dev branch behaviour.

[h2zero]

@DigiH I believe I have resolved the issue now. I just pushed a new commit.

[DigiH]

@DigiH I believe I have resolved the issue now. I just pushed a new commit.

@h2zero This fixes the issue for me that consecutive READ/WRITEs are being exectuted now, even after the devices have been added to the white-list - although with a much higher "success":false rate now. It seems that I need to send every READ/WRITE command at least twice now, before I get a "success":true

BUT after every READ or WRITE command, that particular device is still being excluded from any subsequent scans, not showing up there any longer until a restart.

[h2zero]

It seems that I need to send every READ/WRITE command at least twice now, before I get a "success":true

Try increasing the TTL value, I have not experienced this with my device.

BUT after every READ or WRITE command, that particular device is still being excluded from any subsequent scans, not showing up there any longer until a restart.

I can't reproduce this here, may be an issue for a separate PR.

[h2zero]

@DigiH do you have BLE_FILTER_CONNECTABLE enabled?

[DigiH]

It seems that I need to send every READ/WRITE command at least twice now, before I get a "success":true

Try increasing the TTL value, I have not experienced this with my device.

Even with my same TTL this seems to have gone now, or was just a fluke with the previous testing ;) so can be ignored. Either way, with the response success I'll want to implement an openHAB backend resending of any READ/WRITE commands which failed anyway.

@DigiH do you have BLE_FILTER_CONNECTABLE enabled?

Oh Yes I did ;) thanks for the hint. I had it enabled in my env definition after reading the info in config_BT.h when trying a new BT device, but didn't really see any changes for the device and my openHAB setup, but never deleted/set it to false again after that. With the default back to

'-DBLE_FILTER_CONNECTABLE=0'

all is fine, and the devices are showing back in every scan after a READ/WRITE command. Still not sure if this overwrite should happen though if singular manual READ/WRITE command connections are issued to otherwise non-connectable devices, therefor affecting all subsequent scans. Not affecting me as setting it to true was just a test, but might affect others who need it set to true.

So the immediate parameter for the READ/WRITE commands is working perfectly now. @h2zero Thanks again for all your time and effort you put into this!

[h2zero]

@DigiH You're very welcome and thank you for your help testing. I have pushed, what I believe to be the final iteration, if you wouldn't mind giving it a few tests from your end it would be greatly appreciated.

[DigiH]

if you wouldn't mind giving it a few tests from your end it would be greatly appreciated.

@h2zero tested here today and all working fine with BLE_FILTER_CONNECTABLE disabled. Thanks!

Logging a separate issue though for when BLE_FILTER_CONNECTABLE is enabled for possible future enhancement.

#1144

[h2zero]

Thanks @DigiH, I think I may have a simple solution for that issue as well.

[h2zero]

@DigiH I think that issue is resolved now, just added as a commit to this PR

[DigiH]

@h2zero - that was very quick, thanks :) Closed the related issue as fixed with this latest commit 👍

[1technophile]

Thanks both of you!