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

fix: handle jagged stairstep for LYWSD03MMC humidity #73

Merged
merged 3 commits into from
Mar 10, 2024

Conversation

apaperclip
Copy link
Contributor

I noticed when using the esphome ble proxies for humidity data from LYWSD03MMC sensors, the values would stair step up in .1% increments until .9, then fall back to .0. I did not see this behavior when using the native esphome support for these sensors. In the bluetooth channel in the HA discord, Jc2k recalled that the ble_monitor custom component handled this but it was not ported over. With that in mind I found references to code that worked around this behavior in the ble_monitor custom component and the native esphome sensor for these devices. I've noted the links as comments in the code.

The ble_monitor code accounted for firmware version but I didn't see how to reference firmware version in the parser.py.
I'm happy to adjust but may need some help.

@apaperclip
Copy link
Contributor Author

Arg, I forgot about the tests. Should I simply change the input humidity value to an integer since we'll get an integer back? Or should the tests be broken up since there is logic based on device_type. One test for the new logic where it's supplying a float and getting an integer back, and another supplying a float and getting a float back?

Copy link

codecov bot commented Feb 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.16%. Comparing base (9918c70) to head (8dc5bc9).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   56.07%   56.16%   +0.08%     
==========================================
  Files           6        6              
  Lines         979      981       +2     
  Branches      210      211       +1     
==========================================
+ Hits          549      551       +2     
  Misses        332      332              
  Partials       98       98              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@apaperclip apaperclip changed the title Fix jagged stairstep for LYWSD03MMC humidity fixFix jagged stairstep for LYWSD03MMC humidity Feb 24, 2024
@apaperclip apaperclip changed the title fixFix jagged stairstep for LYWSD03MMC humidity fix: handle jagged stairstep for LYWSD03MMC humidity Feb 24, 2024
@apaperclip
Copy link
Contributor Author

test updated to expect an integer.
here's a graph comparing new (top) to old (bottom)

image
image

@Ernst79
Copy link
Collaborator

Ernst79 commented Mar 10, 2024

Sorry I didn't see your PR earlier, somehow I didn't get an email with a new PR.

You don't need to check the firmware. In BLE monitor, the device LYWSD03MMC is supported both with Xiaomi firmware, and with e.g. BTHome or PVVX custom firmware. In BLE monitor, we only have to apply the "fix" to the sensors that use Xiaomi firmware.

In this repo, we already know that the device is running stock Xiaomi firmware, so no need to check that.

Thanks for the contribution, I'll do a review.

@Ernst79 Ernst79 merged commit efb3229 into Bluetooth-Devices:main Mar 10, 2024
8 checks passed
@apaperclip apaperclip deleted the LYWSD03MMC_humidity branch March 11, 2024 00:07
@apaperclip
Copy link
Contributor Author

Sorry I didn't see your PR earlier, somehow I didn't get an email with a new PR.

You don't need to check the firmware. In BLE monitor, the device LYWSD03MMC is supported both with Xiaomi firmware, and with e.g. BTHome or PVVX custom firmware. In BLE monitor, we only have to apply the "fix" to the sensors that use Xiaomi firmware.

In this repo, we already know that the device is running stock Xiaomi firmware, so no need to check that.

Thanks for the contribution, I'll do a review.

All good, thank you!

@apaperclip
Copy link
Contributor Author

@Ernst79 sorry to tag - but quick question , how/when does this get merged into ha, anything I need to do over there?

@bdraco
Copy link
Member

bdraco commented Mar 15, 2024

You'll need to open a PR to bump the dep in HA if someone doesn't do it for you. In this case home-assistant/core#113013 is already merged and will be in 2024.4.x

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