Bluetooth-Devices / bthome-ble

Parser for BTHome BLE devices
https://bthome.io/
MIT License
71 stars 15 forks source link

feat: remove cipher update #30

Closed Ernst79 closed 1 year ago

Ernst79 commented 1 year ago

As discussed in #20, this PR proposes to remove the cipher.update(b"\x11") in the decryption code for BTHome V2. This will remove the need to apply this in the sensor firmware code as well.

In V1, this cipher.update(b"\x11") will be remained as it was, to keep backward compatibility. V2 isn't used most likely at the moment, as Home Assistant will be updated in the December release with V2 support.

codecov[bot] commented 1 year ago

Codecov Report

Base: 75.39% // Head: 75.33% // Decreases project coverage by -0.05% :warning:

Coverage data is based on head (21b9390) compared to base (849deab). Patch coverage: 75.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #30 +/- ## ========================================== - Coverage 75.39% 75.33% -0.06% ========================================== Files 6 6 Lines 443 442 -1 Branches 65 66 +1 ========================================== - Hits 334 333 -1 Misses 89 89 Partials 20 20 ``` | [Impacted Files](https://codecov.io/gh/Bluetooth-Devices/bthome-ble/pull/30?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/bthome\_ble/bthome\_v2\_encryption.py](https://codecov.io/gh/Bluetooth-Devices/bthome-ble/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2J0aG9tZV9ibGUvYnRob21lX3YyX2VuY3J5cHRpb24ucHk=) | `60.27% <0.00%> (-1.06%)` | :arrow_down: | | [src/bthome\_ble/\_\_init\_\_.py](https://codecov.io/gh/Bluetooth-Devices/bthome-ble/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2J0aG9tZV9ibGUvX19pbml0X18ucHk=) | `100.00% <100.00%> (ø)` | | | [src/bthome\_ble/parser.py](https://codecov.io/gh/Bluetooth-Devices/bthome-ble/pull/30/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2J0aG9tZV9ibGUvcGFyc2VyLnB5) | `80.88% <100.00%> (+0.07%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

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

kzyapkov commented 1 year ago

Fine with us!

Although, now that I look at it, it's a change for the sake of change itself. I necessitates additional if-s to handle, so it may be simpler to just leave it as is. That way, encrypt/decrypt functions can be reused verbatim from v1.

sdrsdr commented 1 year ago

v2 shold be about 'this is the better way to do this' and not 'we made it work like somebody else'. Remove IMHO, or explicitly explain why it is like that in documentation