Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduces delayed BLE actions #3434

Closed
wants to merge 12 commits into from

Conversation

rbaron
Copy link
Contributor

@rbaron rbaron commented May 3, 2022

What does this implement/fix?

This PR introduces delayed BLE actions. This may be useful for triggering BLE writes on non-persistent connections. Using a delayed BLE action will cause a BLE connection to be established, a write to be sent to the specified characteristic and a subsequent disconnection.

This PR also makes BLE action more gracefully handle errors in both delayed (this PR) and eager (#3398) cases. Previously, a write would fail if the connection had not been established at the time of the action execution. Now, the write will be pending until the connection is established, at which point the write will be triggered.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266

Example entry for config.yaml:

# Example config.yaml
esphome:
  name: esphome_ble_led_client
  platform: ESP32
  board: lolin32

esp32_ble_tracker:

ble_client:
  - id: esphome_ble_led_ble_client
    mac_address: D0:90:AB:CD:EF:E2
    maintain_connection: false

switch:
  - platform: template
    name: "ESP32 LED Switch"
    optimistic: true
    turn_on_action:
      - ble_client.ble_write:
          id: esphome_ble_led_ble_client
          service_uuid: d7204ebc-4bc5-4a6a-a7e5-7a86f998b7a9
          characteristic_uuid: 9be42785-94ae-4c1b-aa02-cd87c1c647c8
          value: [0x7f, 0xab]
    turn_off_action:
      - ble_client.ble_write:
          id: esphome_ble_led_ble_client
          service_uuid: d7204ebc-4bc5-4a6a-a7e5-7a86f998b7a9
          characteristic_uuid: 9be42785-94ae-4c1b-aa02-cd87c1c647c8
          value: [0x4b, 0x9d]

logger:
  level: DEBUG

I've set up a minimal ESP32 Arduino-based BLE service to test this PR with in rbaron/ble-led.

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@rbaron
Copy link
Contributor Author

rbaron commented May 3, 2022

Sending this PR as a draft because it builds on top of #3398. I will rebase this PR once #3398 is merged.

@rbaron rbaron changed the title Delayed ble write action Introduces delayed BLE actions May 3, 2022
@rbaron
Copy link
Contributor Author

rbaron commented May 3, 2022

@ppanagiotis, I think I managed to fix the ESP-IDF BLE scan never terminated, rebooting to restore BLE stack... error you mentioned in this comment. I also added a per-ble_client maintain_connection config, so hopefully it doesn't affect other BLE clients anymore.

Let me know if you manage to take this for a spin with your switchbot. I'm curious to see if we get it to work well this time.

@rbaron rbaron mentioned this pull request May 4, 2022
10 tasks
@rbaron
Copy link
Contributor Author

rbaron commented May 4, 2022

I ran some more tests and there is still a bug somewhere:

  1. If I run a single ble_client with maintain_connection: false, the delayed actions work fine. The connection is established and disconnected "on demand" as expected. Repeating the action works fine;
  2. If I run two ble_clients with maintain_connection: false, things work well for both delayed actions;
  3. If I run one ble_client with maintain_connection: true (the default) and one ble_client with maintain_connection: false, the first one starts failing after an initial action is triggered on the second one. I suspect it's a timing issue - when the delayed action prompts a connection for second ble_client, the first ble_client's connection (which had been established and maintained) experiences some problem, but there is no disconnect event (so from ESPHome's perspective, its state is still ESTABLISHED). It feels like the first connection is broken due to the time it takes to process the second (delayed) connection, but that is only apparent when I try to write to its characteristic.

If I run with the Arduino framework, the error on item 3 is simply ESP_FAIL from esp_ble_gattc_write_char. Using the esp-idf framework, the error is ESP_ERR_INVALID_STATE, which comes from esp_gattc_api.c, with a "The connection not created." (although the connection had been created and worked well before triggering the delayed action for the other ble_client). This suggests to me that the connection was broken but its state was not propagated to ESPHome for some reason.

@rbaron
Copy link
Contributor Author

rbaron commented May 4, 2022

I believe the last commit fixes the bug I mentioned in the previous comment. Right now the following multi-ble-client config seems to be working as expected for me:

esphome:
  name: esphome_ble_led_client
  board: lolin32
  platform: ESP32

wifi:
  ssid: !secret wifi_ssid
  password: !secret wifi_password

api:
  password: !secret home_assistant_api

esp32_ble_tracker:

ble_client:
  - id: esphome_ble_led_ble_client
    mac_address: D0:90:AB:CD:EF:E2
    maintain_connection: false
  - id: esphome_ble_led_ble_client2
    mac_address: D0:90:AB:CD:EF:A2

switch:
  - platform: template
    name: "ESP32 LED Switch"
    optimistic: true
    turn_on_action:
      - ble_client.ble_write:
          id: esphome_ble_led_ble_client
          service_uuid: d7204ebc-4bc5-4a6a-a7e5-7a86f998b7a9
          characteristic_uuid: 9be42785-94ae-4c1b-aa02-cd87c1c647c8
          value: [0x7f, 0xab]
    turn_off_action:
      - ble_client.ble_write:
          id: esphome_ble_led_ble_client
          service_uuid: d7204ebc-4bc5-4a6a-a7e5-7a86f998b7a9
          characteristic_uuid: 9be42785-94ae-4c1b-aa02-cd87c1c647c8
          value: [0x4b, 0x9d]

  - platform: template
    name: "ESP32 LED Switch 2"
    optimistic: true
    turn_on_action:
      - ble_client.ble_write:
          id: esphome_ble_led_ble_client2
          service_uuid: d7204ebc-4bc5-4a6a-a7e5-7a86f998b7a9
          characteristic_uuid: 9be42785-94ae-4c1b-aa02-cd87c1c647c8
          value: [0x7f, 0xab]
    turn_off_action:
      - ble_client.ble_write:
          id: esphome_ble_led_ble_client2
          service_uuid: d7204ebc-4bc5-4a6a-a7e5-7a86f998b7a9
          characteristic_uuid: 9be42785-94ae-4c1b-aa02-cd87c1c647c8
          value: [0x4b, 0x9d]

logger:
  level: DEBUG

@ppanagiotis
Copy link

@rbaron It works like a charm with both button and curtains!!! Thank you very much!!
If you need anything more fill free to ask me!
I will keep an eye on battery drain and if I see anything unusual I will tell you.

@Ulrar
Copy link

Ulrar commented Jun 6, 2022

Hi,

This works really well, thanks for the two PRs. I've been using it for a while to manage a bunch of switchbot curtains, got everything working and rendering correctly in HA, including the position slider. The only thing missing is being able to set the position using that slider.

I'm not sure what that would involve but any chance we could make the value field templetable ? I'm not quite sure how to implement the position_action on my template covers without that, my understanding is the pos variable is only accessible from a lambda.

Thanks

@rbaron
Copy link
Contributor Author

rbaron commented Jun 6, 2022

Hi @Ulrar,

That's the direction I have in mind too, but I wanted to start simple and get the basics in, which turned out to be already relatively involved. I am happy to work on it once/if we get these two PRs merged in.

@rbaron
Copy link
Contributor Author

rbaron commented Aug 9, 2022

Rebased against dev since #3398 was merged.

@rbaron
Copy link
Contributor Author

rbaron commented Aug 11, 2022

I've been using this branch for a while, with both delayed and regular ble_clients, and it's working as expected. I've also tested in a esp32 and a esp32-c3 (with #3702 patched in).

@Ulrar has been kindly maintaining a fork of this PR that's usable as an external_component (thanks a lot on behalf of the community!). With the base PR merged, we don't touch const.py anymore, so I believe we could point the external_components directly to this PR with:

external_components:
  - source: github://pr#3434
    components: [ ble_client ]

There are a few things I want to clean up in the code before requesting review, but I would say this is already usable as an external_component.

@Ulrar
Copy link

Ulrar commented Aug 11, 2022

@rbaron Glad to see these going in ! I've been using it since myself too and it's working pretty great.

I expect we'll need to wait for the next release of ESPHome before we can point at this PR directly unless I missed something it's been merged but hasn't been released yet

@rbaron
Copy link
Contributor Author

rbaron commented Aug 11, 2022

You're right @Ulrar. Users either need to:

  1. Run the dev branch and source: github://pr#3434 or
  2. Run the latest release and use https://github.com/Ulrar/esphome-temp.git (as per your comment)

With this commit, the BLE connection to the peripheral will only be
established once the actions are triggered.
Oddly, with two BLEClients, the ESP_GATTC_CONNECT_EVT was called for
each one, which caused their conn_id data member to be overridden.

Following the example from ble_multiple_connect_main.c and pulling the
conn_id from the ESP_GATTC_OPEN_EVT instead (which fires only once for
the correct gattc_if) seems to fix the issue.
The bug was fixed by the parent commit.
@rbaron
Copy link
Contributor Author

rbaron commented Aug 15, 2022

@Ulrar this PR should now work with templatable values, which lets us set the SwitchBot position, in addition to fully open/close 😃. I've been using it like this:

esp32_ble_tracker:

ble_client:
  - id: switchbot_ble_client
    mac_address: F1:E0:37:0D:EE:B5
    maintain_connection: false

cover:
  - platform: template
    name: "Office Curtain Test"
    device_class: curtain
    optimistic: true
    has_position: true
    open_action:
      - ble_client.ble_write:
          id: switchbot_ble_client
          service_uuid: cba20d00-224d-11e6-9fb8-0002a5d5c51b
          characteristic_uuid: CBA20002-224D-11E6-9FB8-0002A5D5C51B
          value: [0x57, 0x0F, 0x45, 0x01, 0x05, 0xFF, 0x00]
    close_action:
      - ble_client.ble_write:
          id: switchbot_ble_client
          service_uuid: cba20d00-224d-11e6-9fb8-0002a5d5c51b
          characteristic_uuid: cba20002-224d-11e6-9fb8-0002a5d5c51b
          value: [0x57, 0x0F, 0x45, 0x01, 0x05, 0xFF, 0x64]
    position_action:
      - ble_client.ble_write:
          id: switchbot_ble_client
          service_uuid: cba20d00-224d-11e6-9fb8-0002a5d5c51b
          characteristic_uuid: cba20002-224d-11e6-9fb8-0002a5d5c51b
          value: !lambda |-
            uint8_t byte_position = 100  * (1.0f - pos);
            ESP_LOGW("POS: ", "pos: %f, byte_position: %d", pos, byte_position);
            return {0x57, 0x0F, 0x45, 0x01, 0x05, 0xFF, byte_position};

It's a lot of YAML for such a simple functionality, but it's generic enough to cater for generic "BLE writes". Hopefully we can wrap some of this into a switchbot component and just have a couple of YAML lines in the future.

Two things preventing me from turning this draft into a PR:

  1. Code is not up to standard, I will refactor things a little bit;
  2. There is some annoying delay between the action trigger and the action being executed. Some delay is expected, as the scanning + connecting takes a few seconds. Rarely, though, specially upon "first execution", it takes upwards of 10s for the whole thing to work, which is not very nice. Anedoctal evidence suggests that the delay is much smaller with the esp32-c3, but it needs more investigation.

@Ulrar
Copy link

Ulrar commented Aug 16, 2022

That's great news ! I've just updated my branch with your changes now and tested it, works like a charm.
Thanks again for all your work on this, it's very much appreciated !

I get the same delay on the first action after a reboot on my doit devkits v1, I've seen somewhere that if you use some arduino code to have the board initiate a pairing request to the switchbot then re-flash ESPHome the delay goes away, but I haven't gotten around to try it myself yet.
It hasn't been a big deal to be honest, 99.9% of actions are quick so I haven't bothered with it.

A SwitchBot component would be great, took a quick look a while back and I'm not that clear on how to go about creating one, but I haven't taken the time to look into it properly yet.

@Ulrar
Copy link

Ulrar commented Aug 17, 2022

Just to confirm, my repo isn't needed anymore with 2022.8.0, this is enough now :

external_components:
  - source: github://pr#3434
    components: [ ble_client ]

I'll delete my repo in a while, I'll give time to anyone who might have been using it to point to his PR instead

@github-actions
Copy link
Contributor

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 16, 2022
@Ulrar
Copy link

Ulrar commented Nov 17, 2022

With the new bluetooth_proxy this may not be as needed anymore, but it's probably still a worthwhile addition

@rbaron
Copy link
Contributor Author

rbaron commented Nov 17, 2022

Thanks @Ulrar, I agree with you. I will close this for now.

@rbaron rbaron closed this Nov 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants