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

MI32: Prevent active BLE read with unencrypted MI-format beacons #22453

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

thewebface
Copy link

Description:

I have various LYWSD03 sensors running ATC firmware that are sending unencrypted MI-format beacons. This is causing Tasmota to constantly try and make active read connections to them. These connections are not necessary as the beacons are unencrypted.

Currently, Tasmota skips the active connection for encrypted beacons if the key is known, this PR make it also skip if the beacons are unencrypted.

I can see the same code is present for the MHOC401 sensor and have applied the same fix, although I haven't tested on this device.

Note: I noticed this issue due to regular reboots caused by BLE disconnect taking > 60s. These no longer happen running this branch.

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.8
  • The code change is tested and works with Tasmota core ESP32 V.3.1.0.241030
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@Jason2866
Copy link
Collaborator

@btsimonh Are you fine with this PR?

@btsimonh
Copy link
Contributor

hmm... the MI code is not something I am overly familiar with. The basic code seems fine :).

However, @thewebface - do your devices respond on the battery indicator LYWSD03_BattNotifyChar?

If so, this could be a good opportunity to check if b6eeb4f has an impact on getting the battery from MI devices....
(see also @Staars 's last comment in #22267)

Would it be possible to investigate if the devices respond correctly when response is set to false, or it they need the default true?

(i.e., just try with bool response = false; replaced with bool response = true; and report if your devices respond one way or the other or both ways)

IF they respond with true, but not with false, we'll need to make some allowance in the BLE32 API to allow the EQ3 driver to specify it wants false, and leave default true for battery requests to MI devices.

@thewebface
Copy link
Author

I've not been doing any active reads at all yet as the sensors are returning the correct battery state in the beacons. I can give it a try though

@thewebface
Copy link
Author

Unfortunately my devices don't appear to support active read of battery state

failed - svc not on device?

@Jason2866
Copy link
Collaborator

Is the PR ready to merge? For me it looks like there are open questions. If so please change to Draft

@thewebface
Copy link
Author

Correct me if I'm wrong @btsimonh but I don't think any of the outstanding questions relate to this PR specifically so as far as I'm aware it should be good to go. I've been running this branch for 2 weeks now and it's been stable.

@btsimonh
Copy link
Contributor

no, the outstanding questions are about whether the BLE driver can get battery notifications from MI devices.
The PR is related to not polling for these if the advertising device is sending unencrypted data. I'm happy with the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants