-
Notifications
You must be signed in to change notification settings - Fork 464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hue: Fix multi-component device initialization race condition #1831
Conversation
Channel deleted. |
Test Results 64 files 402 suites 0s ⏱️ Results for commit 6b62177. ♻️ This comment has been updated with latest results. |
Minimum allowed coverage is Generated by 🐒 cobertura-action against 6b62177 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the logjam
stuff. I wonder if we could get this into the lua_libs
as it seems like it would be universally useful drivers.
@@ -266,6 +273,7 @@ end | |||
---@return HueApiKeyResponse[]? api_key_response nil on err | |||
---@return string? error nil on success | |||
---@return string? partial partial response if available, nil otherwise | |||
---@return ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: what is this indicating, that the function could return any number of additional any
typed arguments? What is the advantage of adding this instead of enumerating the actual return types after the two know error values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was to make the typechecker happy. The actual unpack
here should stop at the three things indicated, but the signature of table.unpack
in LuaLS
's meta typedefs indicates up to 12 return values.
I had the same thought. There are a few more refinements I want to make, but I was thinking it would make sense to port this over to the main |
- Mostly updates type hints/annotations - Fix one place where typechecks caught a missing nil check
e4cc7a4
to
7cdc617
Compare
Using `ipairs` here instead of `pairs` meant that any devices that ended up in this queue were never actually being refreshed. Fixes #1674
7cdc617
to
6b62177
Compare
Type of Change
Checklist
Description of Change
We've identified an issue in the Hue driver related to the way we handled device init on driver startup for child devices that were ahead of the parent bridge in database order.
The race condition primarily manifested in the non-primary component for multi-component devices (such as multi-button remotes) not emitting attribute events or being able to handle capability commands.
The actual bugfix change is here: e4cc7a4
The other two commits are chore work that we tackled in the process of fixing the bug:
Fixes #1674
Summary of Completed Tests
Tested with various multi-button remotes.