From da249022cd92ece32bb731cf7e30f59e9d5fce7d Mon Sep 17 00:00:00 2001 From: HomeOS Bootstrap Date: Sat, 18 Jul 2026 09:51:56 +0200 Subject: [PATCH] fix(shelly): round-trip light brightness/rgb; don't hijack a reused IP (R15) Codex R15 on the local transport: - [P2] A light command set brightness/rgb but the normalizer stored only `on`, so the poll dropped them and the UI/automations went stale. The light state now carries brightness + rgb when the device reports them. - [P2] ShellyLocalOnboarder matched an existing row by IP BEFORE the Shelly id, so a DHCP-reassigned IP could let one Shelly overwrite an unrelated device. Now matches by stable Shelly id first and reuses an IP row only when it's the same (or an unidentified) device. +3 tests (light attrs, housekeeping dropped, IP-reuse no hijack). Suite 71 green. Co-Authored-By: Claude Opus 4.8 --- app/Services/ShellyLocalOnboarder.php | 19 ++++++++++++++----- app/Support/Mqtt/ShellyNormalizer.php | 9 ++++++++- tests/Feature/ShellyHttpTest.php | 14 ++++++++++++++ tests/Feature/ShellyNormalizerTest.php | 17 +++++++++++++++++ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/app/Services/ShellyLocalOnboarder.php b/app/Services/ShellyLocalOnboarder.php index b78ec03..9d69707 100644 --- a/app/Services/ShellyLocalOnboarder.php +++ b/app/Services/ShellyLocalOnboarder.php @@ -26,11 +26,20 @@ class ShellyLocalOnboarder $status = $this->rpc->status($ip); $shellyId = $info['id'] ?? null; - // Reuse an existing row for this device (already added locally, or auto-onboarded via MQTT - // under the same id) so switching to local control never creates a duplicate. - $device = Device::where('config->ip', $ip)->first() - ?? ($shellyId ? Device::where('config->mqtt_prefix', $shellyId)->first() : null) - ?? new Device; + // Reuse an existing row so switching to local control never duplicates. Match on the STABLE + // Shelly id first; only reuse an IP match when that row is the SAME (or an unidentified) + // device — otherwise a DHCP-reassigned IP would let this Shelly overwrite an unrelated one. + $device = $shellyId ? Device::where('config->mqtt_prefix', $shellyId)->first() : null; + + if ($device === null) { + $byIp = Device::where('config->ip', $ip)->first(); + $byIpId = $byIp ? data_get($byIp->config, 'mqtt_prefix') : null; + if ($byIp !== null && (blank($byIpId) || $byIpId === $shellyId)) { + $device = $byIp; + } + } + + $device ??= new Device; $device->fill([ 'name' => $name ?: ($device->name ?: ($info['name'] ?? $shellyId ?? $ip)), diff --git a/app/Support/Mqtt/ShellyNormalizer.php b/app/Support/Mqtt/ShellyNormalizer.php index c514092..f639f1b 100644 --- a/app/Support/Mqtt/ShellyNormalizer.php +++ b/app/Support/Mqtt/ShellyNormalizer.php @@ -34,7 +34,14 @@ class ShellyNormalizer break; case 'light': - $updates[] = ['key' => "light:{$index}", 'type' => 'light', 'state' => ['on' => (bool) ($payload['output'] ?? $payload['ison'] ?? false)]]; + $state = ['on' => (bool) ($payload['output'] ?? $payload['ison'] ?? false)]; + if (array_key_exists('brightness', $payload)) { + $state['brightness'] = (int) $payload['brightness']; + } + if (isset($payload['rgb']) && is_array($payload['rgb'])) { + $state['rgb'] = array_map(fn ($v) => (int) $v, array_slice($payload['rgb'], 0, 3)); + } + $updates[] = ['key' => "light:{$index}", 'type' => 'light', 'state' => $state]; break; case 'cover': diff --git a/tests/Feature/ShellyHttpTest.php b/tests/Feature/ShellyHttpTest.php index f6e42d5..7626744 100644 --- a/tests/Feature/ShellyHttpTest.php +++ b/tests/Feature/ShellyHttpTest.php @@ -60,6 +60,20 @@ class ShellyHttpTest extends TestCase $this->assertSame('http', Device::where('config->mqtt_prefix', 'shellyplus1-abc')->first()->protocol); } + public function test_onboarding_does_not_hijack_a_different_device_on_a_reused_ip(): void + { + // Device A previously held this IP (a different Shelly). After DHCP reassignment a new + // Shelly answers at the same IP — it must NOT overwrite A. + $a = Device::create(['name' => 'A', 'vendor' => 'Shelly', 'protocol' => 'http', 'config' => ['ip' => '10.10.30.78', 'mqtt_prefix' => 'other-shelly']]); + $this->fakeShelly(); // identifies as shellyplus1-abc + + $device = app(ShellyLocalOnboarder::class)->onboard('10.10.30.78'); + + $this->assertNotSame($a->id, $device->id); + $this->assertSame('other-shelly', data_get($a->fresh()->config, 'mqtt_prefix')); + $this->assertSame('shellyplus1-abc', data_get($device->config, 'mqtt_prefix')); + } + public function test_http_driver_controls_via_rpc_and_audits(): void { $this->fakeShelly(); diff --git a/tests/Feature/ShellyNormalizerTest.php b/tests/Feature/ShellyNormalizerTest.php index c7144d2..6564195 100644 --- a/tests/Feature/ShellyNormalizerTest.php +++ b/tests/Feature/ShellyNormalizerTest.php @@ -51,4 +51,21 @@ class ShellyNormalizerTest extends TestCase { $this->assertContains('contact', ShellyNormalizer::PRIMARY_TYPES); } + + public function test_light_carries_brightness_and_rgb_when_present(): void + { + $rgbw = ShellyNormalizer::normalize('light:0', ['output' => true, 'brightness' => 60, 'rgb' => [255, 10, 0]]); + $this->assertSame(['on' => true, 'brightness' => 60, 'rgb' => [255, 10, 0]], $rgbw[0]['state']); + + // A plain on/off light reports no attributes. + $plain = ShellyNormalizer::normalize('light:0', ['output' => false]); + $this->assertSame(['on' => false], $plain[0]['state']); + } + + public function test_housekeeping_components_are_dropped(): void + { + $this->assertSame([], ShellyNormalizer::normalize('sys', ['restart_required' => false])); + $this->assertSame([], ShellyNormalizer::normalize('wifi', ['status' => 'got ip'])); + $this->assertSame([], ShellyNormalizer::normalize('cloud', ['connected' => false])); + } }