Repository navigation
Conversation
|
Invitation URL: |
|
philips-hue_coverage.xml
Minimum allowed coverage is Generated by 🐒 cobertura-action against 1530566 |
Test Results 73 files 561 suites 0s ⏱️ For more details on these errors, see this check. Results for commit 1530566. ♻️ This comment has been updated with latest results. |
varzac
commented
Aug 13, 2026
| ) | ||
| return | ||
| end | ||
| cosock.socket.sleep(backoff()) |
Contributor
Author
There was a problem hiding this comment.
not sure I agree with this change. I'll have to investigate further
Contributor
There was a problem hiding this comment.
The max attempts could potentially be problematic because we only queue scans on connect or device adds so we might end up with no group information. I think the backoff is fine and might actually prevent a hot loop if the api calls don't end up yielding for some reason.
NoahCornell
reviewed
Aug 13, 2026
| ) | ||
| ) | ||
| else | ||
| cosock.socket.sleep(backoff()) |
Contributor
There was a problem hiding this comment.
Same feeling here, the backoff is probably good but not getting the connectivity of the devices could leave some offline until the we reconnect sse or until the device connectivity changes and results in new sse event. This happens in a spawned cosock task so probably no harm in continuing to keep trying.
NoahCornell
reviewed
Aug 13, 2026
| -- without needing to mock rooms/zones REST responses or race the scan's own 45-second debounce | ||
| -- timing against whatever else a test is asserting on the same connection. Defaults to true; | ||
| -- production code never touches this. | ||
| grouped_utils.scanning_enabled = true |
Contributor
There was a problem hiding this comment.
I think its fine but I think ideally the mock bridge server would be able to be configured to default respond to these requests with an empty valid response.
dkjson.decode returns (value, next_position, error_message), but process_rest_response propagated all of pcall's captured return values after decoding, not just the decoded value its own doc comment promises. That means the parse position (e.g. 74 for a 73-byte body) gets returned in the position every caller treats as `err`, so every successful REST call with a JSON body logs a spurious "Error performing <action>: <parse position>". Found via the first integration test to exercise a real, successful JSON-decoded REST response through this path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> philips-hue: fix onmessage misreading json.decode's position as an error table.pack(pcall(json.decode, msg.data)) followed by table.remove(...,1) to strip the pcall success flag left `events, err = table.unpack(...)` capturing dkjson's second return value (the position it stopped scanning at, a non-nil number even on success) into `err` instead of its real third return value. Every SSE message was therefore logged as a JSON parse error and dropped without ever reaching the update/add/delete handling below -- there was no prior test coverage of this path to catch it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
varzac
force-pushed
the
hue-lan-integration-tests
branch
from
August 20, 2026 17:21
1cf2f31 to
38ce6b0
Compare
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
varzac
force-pushed
the
hue-lan-integration-tests
branch
from
August 21, 2026 15:09
38ce6b0 to
c3893ff
Compare
Cover how the batch capability message handler sends commands to the bridge: a batch exactly covering a room becomes one grouped_light request, and everything else (batches of one, lights outside the room, partial room matches, mismatched arguments, commands without a grouped handler) goes out as per-light requests. Adds HueDeviceBuilder:with_room() and fixtures.seed_groups(), which feeds Hue room resources through grouped_utils.group_add once lights are initialized, since group scanning stays disabled in tests. Requires the integration_test `_capability_batch_receive` opt-in from the scripting-engine lan-integration-test-support branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test/fixtures/capture_payloads.lua holds real CLIP v2 SSE events and REST responses recorded by the instrumented driver during physical testing, with every UUID, MAC, IP, bridge id and Zigbee PAN id replaced by a stable fake. Payloads are stored as their original JSON text and decoded on access. HueDeviceBuilder now only expects the init levelRange event for profiles that have switchLevel (so on/off plugs can be built), and exposes `driver_device` for reading or seeding a device's driver-side fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Characterization tests built from the physical test captures: - setLevel 0 and out-of-range setColorTemperature values are clamped to the request bodies the bridge actually received. - A recorded LCT014 add event creates exactly one child device, a recorded smart plug removal deletes it, and recorded resource events the driver doesn't model (homekit, geolocation, behavior_script, ...) are ignored. - After a driver restart an onboarded light only receives init and is refreshed once, after the event stream's connectivity check. - With two bridges on the network, only the one that returns an API key gets a device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ur mode Two issues seen in the physical test captures after setColor: - In xy colour mode the bridge reports `mirek: null, mirek_valid: false`, and the driver fell back to the default mirek and emitted a 6500K colorTemperature event alongside the new hue/saturation. Skip the colour temperature event when mirek_valid is false. - SSE colour updates only carry `xy`, and the driver stored their missing gamut over the light's real one, so later setColor commands and colour reports used the default gamut. Only store a gamut that is present, and fall back to the stored one otherwise. The refresh path also stored `gamut_type` (e.g. "C") in the GAMUT field, which expects a gamut table; it now stores the table. The emitter overwrote it immediately, so this had no visible effect. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s is off A grouped setLevel always sent a grouped_light `on` request before the brightness request, doubling grouped traffic (which the bridge rate-limits) for rooms that were already on; physical testing showed two grouped PUTs for every grouped setLevel. Check the lights' cached switch state first, as the individual setLevel handler does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Light init built the device's service map (resource id -> type) but never stored it in driver.services_for_device_rid, which is what routes a zigbee_connectivity SSE update to the device that owns it. Until a bridge scan happened to fill the map in, connectivity changes for lights were dropped: a light losing its Zigbee connection stayed online. Store the map as the button and sensor init paths already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The EventSource calls onerror for every failed connection attempt, about every 3 seconds while the bridge is unreachable, and each call marked the bridge and every child offline again. During physical testing one outage produced 315 offline updates in about two and a half minutes (45 failed attempts x 7 devices). Report the outage once and re-arm when the stream reconnects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Light init always emitted switchLevel.levelRange, which an on/off smart plug's profile doesn't have; the hub dropped it with a "does not support capability Switch Level" warning on every plug init. Only emit it when the profile supports switchLevel, and add tests for a plug built from its recorded light and device resources. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the test that called the EventSource's onerror directly with one
where the bridge drops the stream and refuses the next two reconnects
("No route to host", as recorded in physical test scenarios 8 and 9),
using the integration test framework's new fail_connects.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alive With the integration test framework now giving each connection its own mock entry, cover physical test scenario 6: a PUT sent on a connection the bridge had already closed is resent on a new connection. The outage test also checks that a second outage after reconnecting is reported again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Using the integration test framework's new prepend/times expectations, cover the bridge answering the next two event stream reconnects with 503: devices are marked offline once and come back after the third attempt. The resource event tests now use a single-use SSE handshake, which the framework can deliver events after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Register the platform's samsungim.hueSyncMode definition (fetched from the SmartThings API) for all Hue tests, so recorded light resources no longer need their `mode` stripped and the resulting Hue sync mode events are checked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Use the integration test framework's assert_sent_json and count_requests instead of pattern-matching raw request bytes, so the assertions no longer depend on JSON key order, and drop the batch tests' hand-written request counter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…istory Replace the outage tests' monkeypatched device_offline with the integration test framework's device status history, and also check that the bridge is marked online exactly once when the stream reconnects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace hand-rolled wait loops with integration_test.wait_until, which fails with a description of what didn't happen instead of falling through to a later assertion, and check the keep-alive resend by which connection each request went out on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
create_hue_scenario's REST connection now accepts GET and PUT unless the test asks for a separate PUT connection, so the plug tests no longer need one to send commands. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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.
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests