Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #89 +/- ##
==========================================
- Coverage 91.55% 91.33% -0.23%
==========================================
Files 11 11
Lines 1066 1142 +76
Branches 132 138 +6
==========================================
+ Hits 976 1043 +67
- Misses 66 74 +8
- Partials 24 25 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sopelj
left a comment
sopelj
left a comment
There was a problem hiding this comment.
Hey, sorry for the delay and thanks for the PR. I have a few questions/requests, but it looks good overall. Would be be possible to rebase with main?
If you would rather, I can make changes based on your PR instead. Thanks!
| LiquidState.COOLING: "Cooling", | ||
| LiquidState.HEATING: "Heating", | ||
| LiquidState.TARGET_TEMPERATURE: "Perfect", | ||
| LiquidState.TARGET_TEMPERATURE: "Ready", |
There was a problem hiding this comment.
I'm not opposed to this change, but it isn't really pertinent in this PR. It will break things in the Home Assistant integration as well.
| def can_write(self) -> bool: | ||
| """Check if the mug can support write operations.""" | ||
| return self.data.udsk is not None | ||
| return self._session_prepared |
There was a problem hiding this comment.
Perhaps instead of _session_prepared we could call it _authenticated or something. I think it makes more sense for checks.
| try: | ||
| dsk = await self._read_without_lock(MugCharacteristic.DSK) | ||
| except BleakError as error: | ||
| logger.debug("Unable to read DSK before pairing: %s", error) | ||
| if not await self._pair_for_session(): | ||
| return False | ||
| paired = True | ||
| try: | ||
| dsk = await self._read_without_lock(MugCharacteristic.DSK) | ||
| except BleakError as retry_error: | ||
| logger.debug("Unable to read DSK after pairing: %s", retry_error) | ||
| return False |
There was a problem hiding this comment.
Is there any reason we need to try to read before pairing? The operation is very quick and errors are ignored. Otherwise we can just call pair in the setup like before and not have to perform each operation twice.
| await self._prepare_session() | ||
| await self.subscribe() | ||
|
|
||
| async def _read_without_lock(self, characteristic: MugCharacteristic) -> bytearray: |
There was a problem hiding this comment.
These methods are a good idea, but I'm not sure they are very useful. They are each only called once with one uuid. We can just call self._client.write_gatt_char or self._client.read_gatt_char in those edge cases
|
|
||
| async def _prepare_session(self) -> bool: | ||
| """Prepare the official Ember DSK/UDSK session when possible.""" | ||
| self._session_prepared = False |
There was a problem hiding this comment.
Can't this just be defined as the default value on the object or in init?
| async def set_udsk_raw(self, udsk: bytes | bytearray) -> None: | ||
| """Write a raw UDSK value.""" | ||
| await self._write(MugCharacteristic.UDSK, bytearray(udsk)) | ||
| self.data.udsk = decode_byte_string(udsk) |
There was a problem hiding this comment.
This can probably replace or be merged with set_udsk and this is the only valid way of doing it.
This branch has not been deployed
2 participants
Hi there, first attempt to solve #88