Conversation
Local discovery probed one candidate address per network interface sequentially, each with a one second timeout, so every 10.x interface that is not a Homey added a full second to getHomeys. Virtualization adapters commonly add several of those. Interfaces sharing a subnet also produced a duplicate probe for the same address. Collect the unique candidate addresses first, then probe them concurrently. With four such interfaces, two of them in one subnet, this drops from 4006ms to 1004ms; with eight it drops from 9012ms to 1005ms. The total is now bounded by the timeout instead of scaling with the interface count.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation is focused, preserves existing discovery behavior, and includes appropriate regression coverage.
Pull request overview
Parallelizes and deduplicates USB Homey discovery probes, reducing discovery latency.
Changes:
- Collects unique candidate IPs before probing.
- Probes candidates concurrently with unchanged timeout behavior.
- Adds coverage for concurrency and subnet deduplication.
File summaries
| File | Description |
|---|---|
lib/AthomApi.js |
Deduplicates and parallelizes USB discovery probes. |
tests/lib/athom-api.fetch.test.mjs |
Verifies concurrent, unique probing. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What
Local Homey discovery probes its candidate USB addresses concurrently, and only once per subnet.
Why
getHomeys()looks for a USB-connected Homey by pinging10.x.y.1for every network interface whose address starts with10.. Those probes ran one after another with a one second timeout each, so every such interface that is not a Homey added a full second to any command that has to resolve a Homey first, includinghomey api ...,homey listandhomey select.Virtualization tools routinely add a couple of
10.xadapters to a machine, and none of them ever answer. Two interfaces in the same subnet made it worse: both map to the same candidate address, so that address was probed twice, each time for the full timeout.How
Collect the unique candidate addresses first, then run the probes with
Promise.all. The timeout, the request and the id matching are unchanged, so the cost is now bounded by the one second timeout instead of scaling with the number of interfaces.Testing
Measured with
os.networkInterfacesstubbed to report dead10.xadapters and the rest ofgetHomeysstubbed out:End to end on a machine with three such interfaces (two from a virtualization tool, one from a VPN tunnel), running
homey api raw --path /api/manager/system/against a real Homey over the cloud strategy: 3.05s, 3.12s, 3.14s before, 1.94s, 1.97s, 1.99s after.tests/lib/athom-api.fetch.test.mjsgains a case that holds every probe open until all of them have started, so a sequential implementation cannot reach the second address. It fails on the previous implementation and needs no timers.develop(Docker host address and completion fixtures).eslint --max-warnings=0andprettier --checkare clean.