fix: detect movement by currentPosition delta, not stale "operation" field - #26
Open
nrocha22 wants to merge 1 commit into
Open
fix: detect movement by currentPosition delta, not stale "operation" field#26nrocha22 wants to merge 1 commit into
nrocha22 wants to merge 1 commit into
Conversation
The gateway's operation field reports the last command sent, not the current motion state. Using it for positionState() caused HomeKit to show "Opening.../Closing..." indefinitely after movement completed. The natural alternative currentState is unreliable: 1149 captured packets during real movement all reported currentState=4. Use delta of currentPosition between consecutive polls instead. Also simplify the TargetPosition sync in updateAccessory: the previous transition check (newState !== prevState && newState === STOPPED) never fired because positionState() always returned the same value, leaving Home's global tile stuck on a perpetual progress animation. Closes jhurliman#25
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
1 participant
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.
Closes #25.
The gateway's
operationfield reports the last command sent, not the current motion state. This caused two visible bugs:TargetPosition-sync code intended to fix this never fired (its precondition compared two values both derived from the brokenoperationfield).Changes
positionState()now comparescurrentPositionbetween the new and previous status. Delta ≠ 0 → movement (direction from sign). Delta = 0 → STOPPED. Honorsconfig.invert.TargetPositionsync inupdateAccessory()is simplified: always sync when the blind is detected as STOPPED. TheHoldPositionwrite was redundant once the blind is already at rest and is removed.Validation
Tested on 7 RollerBlind devices over ~30 minutes of normal use plus an automation run:
currentState=4is invariant during movement is documented in the linked issue.Behavior change
On the very first poll for a freshly-loaded accessory (no prior status),
positionState()returns STOPPED instead of inferring from the staleoperationfield. More accurate and matches the "at rest until proven otherwise" assumption.No regression on
config.invertDirection calculation preserved:
(this.config.invert ? !increasing : increasing).