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

feat: rework sectors, fix pressures, and fix misidentification of IO 9 as 8 #30

Merged
merged 7 commits into from
Jan 8, 2023

Conversation

Lash-L
Copy link
Collaborator

@Lash-L Lash-L commented Jan 8, 2023

Okay this one is kind of spread out. I tired to make sure lint wont get made at me, but we will see.

Pressures

  • When you push a button while you have high pressure, it was previously giving an unknown pressure, this has been resolved to give the correct button pressed.
  • pressure 122 is hitting a power button ( at least on my io series)
  • relating to issue 85115

Sectors

  • this should potentially be renamed, as people seem to be confused into thinking this means what sector of your mouth you are brushing, in reality, it is what sector of the 2 minute clock you are on, first 30 seconds, 2nd 30 seconds, etc.
  • When you pause, the sector will go up by 8, 16, or 24 and then continue counting sectors from there.
  • It is possible for a user to continue brushing after succeeding, 41 -> 43 represent the 3 extra sectors after
    a successful brushing session.
  • succeeding is classified as stopping the toothbrush after the 2 minute mark
  • Oddly enough, Sector 4 is marked as 7.
  • relating to issue 85115

Io 8/9

  • If we look at issues #82939 IO 8 reports as Smart Series 7000 #24 , and 81159 we can see io 8/9 device bytes have some overlap. there is a potential solution by using the last bytes for io 8, but we need more data to be sure if that is a real solution.

Offline State for io series

  • Multiple issues such as 81901 discuss wishing that the sensors didn't go 'unavailable' after completing. through my limited research, it seems like bluetooth.async_track_unavailable may be the solution in home assistant. Once the device goes unavailable - set all of the states to 'offline' or 0 or something similar. This offline state just allows that functionality to happen as there will be a state that can be set. I haven't even attempted this(but I plan to try) because developing on core seems like it will be a challenge, so I am not sure if it is a real solution, but I thought I would add in the functionality that is needed here, as it wont cause any kinds of issues.

Lash-L added 4 commits January 8, 2023 15:24
added pressures for 122,182, and 186 relating to buttons
sectors are not what sector of your mouth the brush is in, but rather what time sector you are in, first, second, third, or forth 30 second sector
due to incorrect identifying one as the other, until we have more data, both models should be grouped
not a real state, but will be useful as an alternative to 'unknown'
@codecov
Copy link

codecov bot commented Jan 8, 2023

Codecov Report

Base: 95.78% // Head: 95.91% // Increases project coverage by +0.12% 🎉

Coverage data is based on head (6824ff8) compared to base (a1426b5).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 6824ff8 differs from pull request most recent head c3de8c5. Consider uploading reports for the commit c3de8c5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   95.78%   95.91%   +0.12%     
==========================================
  Files           2        2              
  Lines          95       98       +3     
  Branches        6        7       +1     
==========================================
+ Hits           91       94       +3     
  Misses          2        2              
  Partials        2        2              
Impacted Files Coverage Δ
src/oralb_ble/__init__.py 100.00% <100.00%> (ø)
src/oralb_ble/parser.py 95.69% <100.00%> (+0.14%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/oralb_ble/parser.py Outdated Show resolved Hide resolved
@Lash-L
Copy link
Collaborator Author

Lash-L commented Jan 8, 2023

This has been a good introduction for me into the world of BLE.

I'd like to start understanding development for HA, if you have a second, I'd appreciate you tell me if my steps are correct.

  1. Fork ha core and launch it

  2. Update manifest to newest version of oralb-ble here

  3. Update documentation here

  4. Attempt to make bluetooth.async_track_unavailable work if possible

  5. add new test and test

  6. Create PR on home assistant core

this code should run as long as brush is not running
@bdraco
Copy link
Member

bdraco commented Jan 8, 2023

This has been a good introduction for me into the world of BLE.

I'd like to start understanding development for HA, if you have a second, I'd appreciate you tell me if my steps are correct.

  1. Fork ha core and launch it
  2. Update manifest to newest version of oralb-ble here
  3. Update documentation here
  4. Attempt to make bluetooth.async_track_unavailable work if possible
  5. add new test and test
  6. Create PR on home assistant core

That sounds like a reasonable path to me. Be sure to link the docs changes in the PR description.

@bdraco bdraco changed the title feat: reworking sectors, fixing pressures, and fixed misidentification of io 9 as 8. feat: reworking sectors, fixing pressures, and fixed misidentification of io 9 as 8 Jan 8, 2023
Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. For future PRs it would be nicer to split feat and fix commits into separate PRs

src/oralb_ble/parser.py Outdated Show resolved Hide resolved
src/oralb_ble/parser.py Outdated Show resolved Hide resolved
@bdraco bdraco changed the title feat: reworking sectors, fixing pressures, and fixed misidentification of io 9 as 8 feat: rework sectors, fix pressures, and fix misidentification of IO 9 as 8 Jan 8, 2023
@bdraco bdraco merged commit 7000169 into Bluetooth-Devices:main Jan 8, 2023
@Lash-L
Copy link
Collaborator Author

Lash-L commented Jan 8, 2023

Thanks. For future PRs it would be nicer to split feat and fix commits into separate PRs

Heard. Will make sure to do so in the future

@bdraco
Copy link
Member

bdraco commented Jan 8, 2023

Also since you are making significant changes you should add yourself to codeowners for the integration in HA

https://github.com/home-assistant/architecture/blob/master/adr/0008-code-owners.md

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.

2 participants