fix: Make the docs and the code say the same thing - #19
Merged
Merged
Conversation
The config flow's first screen still told users that relays, power and energy stay with the official BleBox integration. That stopped being true when this became a full replacement: it creates those entities itself. It was the only text shown when adding a device by IP, it shipped in both languages, and it steered people into keeping the official entry, which produces two of every entity and two device rows. The discovery screen carried the same events-only framing, and so did four module docstrings, one of which stated a design rule that its own file breaks, so a contributor checking the UI text against the code found it corroborated rather than contradicted. The backlight offered a brightness slider that did nothing. Colour mode RGB is Home Assistant's way of saying an entity supports brightness, but turn_on read only the colour and discarded the rest. The device stores no brightness field, only an rrggbb value, so brightness now lives where the hardware already keeps it: in the colour itself, since 804000 is ff8000 at half power. Withdrawing the claim instead was not available. Every colour mode implies brightness, so the only way to stop advertising it would have been to drop to on and off and lose the colour control the device really does have. The device API register was missing four endpoints, one of them an undocumented write that reconfigures the device's network and whose answer the code trusts. The setup guide promised an event attribute that only provisioned URLs carry, described a slot shortage message as reporting numbers it does not report, and never mentioned that automatic mode puts back a callback deleted in the wBox app, which is deliberate and would be baffling undocumented. The input count was documented as changeable by re-adding the device, which aborts. Dead code removed: a firmware check nothing called, kept green by its own tests, a trigger parameter no caller ever set, an unused constraint accessor and a brute-forced inverse of a function that already exists. The protocol constants that decode what a device reports are kept, since those are knowledge rather than dead weight, and the reason is now written down. The relay placeholder constants are deleted, but what they recorded, that the device substitutes a constant rather than the relay state, is kept in prose where it cannot look accidental. Per-relay entity construction and the relay field walk were written out three and four times. One copy of the countdown predicate had already drifted from the one the capability cache uses, which would have given a device restarted while offline the wrong sensors.
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.
1. The config flow contradicted the integration
config.step.userstill said relays, power and energy stay with the official BleBox integration. That stopped being true when this became a full replacement: it creates the relay switch and the power and energy sensors itself. It was the first and only text shown when adding a device by IP, it shipped in both languages, and it steered users into keeping the official entry running, which the README says produces two of every entity and two device rows.Rewritten in all three language files, along with the discovery screen, which carried the same events-only framing and is the more common entry point.
Further drift found while checking:
config.step.inputs.descriptionsaid the input count could be changed later by re-adding the device. Re-adding hits_abort_if_unique_id_configuredand aborts asalready_configured; there is no reconfigure step. Corrected.2. The backlight offered a brightness slider that did nothing
Reported from the field during this work: colour works, brightness does not.
ColorMode.RGBis Home Assistant's contract for "supports brightness", butasync_turn_onread onlyrgb_color, so abrightnessargument passed schema validation and was silently discarded, and the entity publishedbrightness: Nonewhile on.Confirmed against live hardware that the device stores no brightness field at all:
Implemented rather than withdrawn, because withdrawing was not available: every colour mode is in
COLOR_MODES_BRIGHTNESS, so the only way to stop advertising brightness isColorMode.ONOFF, which drops the colour control the device really does have.Brightness lives where the hardware already keeps it, in the colour:
804000isff8000at half power.brightnessis the largest component,rgb_coloris the stored colour normalised back to full, andturn_onfolds them together, taking the missing half from what the device holds. Verified against the real stored value:Plain
turn_on()with no arguments still writes only{"enabled": 1}, so it does not rewrite the colour on every call.One inherent caveat: scaling is lossy in one direction, so a deliberately dark colour reads back as a brighter one at low brightness. The device cannot distinguish dim orange from dark brown. That is how every RGB-only light in Home Assistant behaves.
3. The device API register was incomplete
Missing four endpoints the code calls:
/info,GET /api/device/network,POST /api/device/setandGET /api/ota/check. The full list was re-derived fromblebox_actions.pyrather than taken from the review; those four were the only gaps.POST /api/device/setmatters most, an undocumented write that reconfigures the device's network and whose response the code trusts. The "both set endpoints answer with the resulting state" section named two of the three writes that do.4. Setup guide inaccuracies
power_wwas promised unconditionally, but the placeholder is only appended to URLs the integration provisions. A manually pasted URL has no query string and yields nothing.itemsLimit: 30. Every test fixture uses 6 for convenience, which is what made it look unsupported. Now qualified with the firmware it was measured on.5. Four module docstrings described the pre-replacement architecture
__init__.pysaid the official integration keeps ownership of relay, power and energy whilePLATFORMSbelow lists six platforms.sensor.pystated a design rule that two classes in the same file break.switch.pysaid "neither of these is exposed by the official integration" in a module with three kinds of switch.coordinator.pyomitted the fast poll that is now its primary job.This matters more here than in most projects: the commenting style is deliberately verbose about device quirks, so readers learn to trust the prose and have no way to spot the stale paragraphs. A grep sweep found three more drifting mentions, now fixed; the rest were verified still true and left.
6. Dead code and duplication
Deleted after confirming each unreachable: a firmware check nothing called (kept green by its own tests, which is exactly the false confidence the audit named), a trigger parameter no production caller ever set, an unused constraint accessor, a brute-forced inverse of an existing function, and two unread config-flow fields.
Kept deliberately, with the reason now written down:
TRIGGER_ANY_EDGEandTRIGGER_PERIODICare a decoding table for slots read back off a device, not a menu of what gets written. Someone reading a diagnostics dump needs to knowtriggerType: 19is a timer and not a broken binding.The relay placeholder constants were deleted, but what they recorded, that the device substitutes a constant rather than the relay state, is kept in prose. A constant nobody reads implies it is used; a sentence explaining why it is not cannot.
Per-relay construction appeared three times and the field walk four. Factoring those out caught a live hazard: one copy of the countdown predicate had drifted from the one
capability_signatureuses, so a device restarted while unreachable would have got the wrong countdown sensors. The capability checks in eachasync_setup_entryare deliberately left alone.Testing
226 passing, coverage 98% to 99%,
ruffclean, all three language files key-for-key in sync.Behaviour changes (backlight brightness) have new tests. Everything else is either comment-only or a behaviour-preserving refactor where the existing tests are the evidence, and the report says plainly which is which.
Backlight brightness is verified against the real device's stored colour but not yet exercised on hardware end to end.