Fix Hydrojet V02 bubbles handling - #115
Open
OdynBrouwer wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves Hydrojet V02 bubbles handling so the MEDIUM level is detected reliably (including wave_state=41) and attempts to make MEDIUM selection from OFF more reliable by introducing a toggle sequence.
Changes:
- Extend Hydrojet bubbles value mapping to treat additional reported values (e.g.,
41, and normalized50) as MEDIUM. - Add a HIGH→MEDIUM command sequence when requesting MEDIUM from OFF (implemented in the AWS IoT bubbles setter path).
- Add tests for
wave_state=41handling and for the OFF→MEDIUM toggle behavior; add additional debug logging around wave mapping and selection.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
tests/test_aws_iot_api.py |
Adds unit tests for wave_state=41 mapping and the OFF→HIGH→MEDIUM toggle sequence. |
custom_components/bestway/select.py |
Adds debug logging and changes how the current bubbles option is derived from device.attrs["wave"]. |
custom_components/bestway/bestway/model.py |
Extends HYDROJET_BUBBLES_MAP to accept multiple MEDIUM read values. |
custom_components/bestway/aws_iot/api.py |
Adds debug logging and implements the toggle sequence for MEDIUM-from-OFF; adds additional wave_state logging in normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+129
to
+132
|
|
||
| bubbles_level = self.entity_description.get_fn(wave_value) | ||
| _LOGGER.debug("🔵 current_option: mapped to BubblesLevel=%s", bubbles_level) | ||
|
|
Comment on lines
+128
to
+152
| _LOGGER.debug("🔵 current_option: device.attrs['wave']=%s", wave_value) | ||
|
|
||
| bubbles_level = self.entity_description.get_fn(wave_value) | ||
| _LOGGER.debug("🔵 current_option: mapped to BubblesLevel=%s", bubbles_level) | ||
|
|
||
| option = _BUBBLES_OPTIONS.get(bubbles_level) | ||
| _LOGGER.debug("🔵 current_option: final option=%s", option) | ||
| return option | ||
| return None | ||
|
|
||
| async def async_select_option(self, option: str) -> None: | ||
| """Change the selected option.""" | ||
| _LOGGER.debug("🔵 async_select_option: user selected option=%s", option) | ||
|
|
||
| bubbles_level = BubblesLevel.OFF | ||
| if option == _BUBBLES_OPTIONS[BubblesLevel.MEDIUM]: | ||
| bubbles_level = BubblesLevel.MEDIUM | ||
| elif option == _BUBBLES_OPTIONS[BubblesLevel.MAX]: | ||
| bubbles_level = BubblesLevel.MAX | ||
|
|
||
| _LOGGER.debug("🔵 async_select_option: mapped to BubblesLevel=%s", bubbles_level) | ||
| await self.entity_description.set_fn( | ||
| self.coordinator.api, self.device_id, bubbles_level | ||
| ) | ||
| _LOGGER.debug("🔵 async_select_option: API call complete, requesting refresh") |
Comment on lines
127
to
128
| # Map to V01 Airjet format (0/50/100) for AIRJET_V01_BUBBLES_MAP compatibility | ||
| # Note: Hydrojet uses 40 for MEDIUM, so no mapping needed there |
Comment on lines
+868
to
+886
| # Some V02 Hydrojet devices ignore a direct MEDIUM (40) command when currently OFF. | ||
| # Physical button cycles: OFF -> HIGH -> MEDIUM -> OFF. To reliably reach MEDIUM | ||
| # from OFF, send a HIGH (100) toggle first, wait briefly, then send MEDIUM (40). | ||
| try: | ||
| cached = self._state_cache.get(device_id) | ||
| current_wave = None | ||
| if cached and isinstance(cached.attrs, dict): | ||
| current_wave = cached.attrs.get("wave") | ||
| except Exception: | ||
| current_wave = None | ||
|
|
||
| # If requesting MEDIUM but currently OFF, perform a toggle sequence | ||
| if target_value == 40 and (current_wave is None or int(current_wave) == 0): | ||
| _LOGGER.debug( | ||
| "🔵 airjet_v01_spa_set_bubbles: current_wave=%s, sending toggle sequence HIGH->MEDIUM", | ||
| current_wave, | ||
| ) | ||
| # Send HIGH first | ||
| await self.set_device_state(device_id, {"wave_state": 100}) |
Comment on lines
+368
to
+371
| with patch("custom_components.bestway.aws_iot.api.asyncio.sleep", new=AsyncMock()): | ||
| await aws_api.hydrojet_spa_set_bubbles("device1", # type: ignore[arg-type] | ||
| __import__("custom_components.bestway.bestway.model", fromlist=["BubblesLevel"]).BubblesLevel.MEDIUM | ||
| ) |
Comment on lines
+348
to
+361
| aws_api.devices = { | ||
| "device1": BestwayDevice( | ||
| protocol_version=2, | ||
| device_id="device1", | ||
| product_name="AIRJET", | ||
| alias="Test Spa", | ||
| mcu_soft_version="unknown", | ||
| mcu_hard_version="unknown", | ||
| wifi_soft_version="unknown", | ||
| wifi_hard_version="unknown", | ||
| is_online=True, | ||
| backend="aws_iot", | ||
| product_id="T53NN8", | ||
| ) |
Comment on lines
+129
to
+134
| _LOGGER.debug("🔵 normalize_aws_state: INPUT wave_state=%s", wave_state) | ||
| if wave_state == 40: | ||
| wave_normalized = 50 # Map V02 MEDIUM (40) → V01 Airjet MEDIUM (50) | ||
| else: | ||
| wave_normalized = wave_state # 0 and 100 are same in both | ||
| _LOGGER.debug("🔵 normalize_aws_state: OUTPUT wave_normalized=%s", wave_normalized) |
OdynBrouwer
changed the title
cdpuk
requested changes
Jul 4, 2026
cdpuk
left a comment
cdpuk
left a comment
Owner
There was a problem hiding this comment.
The fix in this PR has been partially implemented in #120. This PR still potentially contains some value in sorting out the direct bubbles transition to MEDIUM, but a rebase is requried to get things in order.
I'm not sure where all the Copolit review comments came from - that's not something I recall enabling, however some of the comments do appear to be valid.
|
|
||
| V02 uses same toggle approach as Airjet V02. | ||
| """ | ||
| _LOGGER.debug( |
Owner
There was a problem hiding this comment.
A lot of log messages have been added in this PR. This appears to be to aid "debugging by print statements" in the absence of a proper debugger connection. Furthermore, they all contain emojis, which is inconsistent with the existing style, and strongly hints at an unreviewed AI generated change. Please can these be stripped back to an appropriate level.
| .coverage | ||
| .idea | ||
| node_modules | ||
| tests/LOGS/ |
Owner
There was a problem hiding this comment.
Is this specific to your environment? I don't recall ever seeing this generated.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes Hydrojet V02 bubble handling so MEDIUM is detected reliably and can be selected from OFF.
Changes: