fix(mqtt): withhold the oven setpoint when no cycle is set - #52
Merged
Conversation
With no cycle set the oven reports x.com.samsung.da.desired = 0, and flatten() published that straight through as target_temp_c. Home Assistant rejects it against the Number entity's declared 30-270 range on every publish, which produced 66,899 log errors over three weeks: Invalid value for number.samsung_oven_setpoint: 0 (range 30.0 - 270.0) 0 is not a 0 degree target, it is the absence of a setpoint, so treat anything outside the settable band as absent. null lands as unknown on both the Number and the Setpoint sensor, the way completion_minutes already reads when the oven is idle. _setpoint applied these bounds on the write side already; only the read path was missing them. Adds the first tests for the sample descriptors. One of them pins a non-obvious asymmetry: the write path snaps to the 5 degree step grid before bounds-checking, so 29 commits as 30 and 271 as 270, and only 0 is refused outright. The invariant that has to hold is the weaker one, that every value the write path commits is one flatten() will publish back, or a write appears to succeed and then reads as unknown.
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.
With no cycle set the oven reports
x.com.samsung.da.desired = 0, andflatten()published that straight through astarget_temp_c. Home Assistant rejects it against the Number entity's declared 30-270 range on every publish:66,899 occurrences over three weeks on my own oven, which is how long it took me to notice.
0is not a 0 degree target, it is the absence of a setpoint, so anything outside the settable band is now withheld.nulllands as unknown on both the Number and the Setpoint sensor, the waycompletion_minutesalready reads when the oven is idle._setpointapplied these bounds on the write side already; only the read path was missing them.Tests
First tests for the sample descriptors, which is why this went unnoticed for three weeks.
One of them pins a non-obvious asymmetry I got wrong on the first attempt. The write path snaps to the 5 degree step grid before bounds-checking, so 29 commits as 30 and 271 as 270, and only 0 is refused outright. Asserting that both paths reject the same inputs fails, and the code is right. The invariant that has to hold is the weaker one: every value the write path commits is one
flatten()will publish back, otherwise a write appears to succeed and then reads as unknown.Validation
main(after fix(mqtt): retire session workers on reconnect #44 and docs(protocol): clarify OwnerPSK vector scope #46)temp=0/None°Cwhile idle instead oftemp=0/0°CScope
mqtt_demo/only, so nothing here ships in thesmartthings-localpackage. No file overlap with #36, #47, #48 or #49, and all four merge clean against it.