diff --git a/Home Connect Cloud/api-test.php b/Home Connect Cloud/api-test.php index 7839f19..d844203 100644 --- a/Home Connect Cloud/api-test.php +++ b/Home Connect Cloud/api-test.php @@ -7,8 +7,12 @@ trait TestAPI { public $selectedProgram = ''; + //Counts API GET requests so tests can assert that no retry loop burns the rate limit. + public static $requestCount = 0; + public function getRequest(string $endpoint) { + self::$requestCount++; preg_match('/homeappliances\/.+\/programs\/selected/', $endpoint, $matches); if ($matches) { return file_get_contents(__DIR__ . '/../tests/' . $endpoint . '/' . $this->selectedProgram . '.json'); diff --git a/Home Connect Cloud/module.php b/Home Connect Cloud/module.php index 22d10c6..ae4b574 100644 --- a/Home Connect Cloud/module.php +++ b/Home Connect Cloud/module.php @@ -102,8 +102,16 @@ public function ForwardData($Data) public function ReceiveData($JSONString) { $this->SendDebug('Receive', $JSONString, 0); + + // A 429 can also arrive through the event stream itself (the /events + // request is rejected). Feed it into the same rate-limit machinery so the + // reconnect logic backs off instead of hammering /events every 60s. + if ($this->applyRateLimitFromStream($JSONString)) { + return; + } + $data = json_decode($JSONString, true); - switch ($data['Event']) { + switch ($data['Event'] ?? '') { case 'KEEP-ALIVE': { $this->SendDebug('KeepAlive', 'OK', 0); $this->SetBuffer('KeepAlive', time()); @@ -154,6 +162,15 @@ public function ForceRegisterServerEvents() public function RegisterServerEvents() { + // Do not (re)connect the event stream while we are rate limited - every + // reconnect hits /events and counts against the daily quota. Defer the + // reconnect to the moment the limit expires instead. + if ($this->isRateLimitActive()) { + $remaining = $this->ReadAttributeInteger('RateLimitUntil') - time(); + $this->SendDebug('RegisterServerEvents', sprintf('Rate limit active, defer reconnect by %ds', $remaining), 0); + $this->SetTimerInterval('Reconnect', max(1, $remaining) * 1000); + return; + } try { $url = self::HOME_CONNECT_BASE . 'homeappliances/events'; $this->SendDebug('url', $url, 0); @@ -179,6 +196,10 @@ public function RegisterServerEvents() public function CheckServerEvents() { + // While rate limited the missing keep-alive is expected - do not reconnect. + if ($this->isRateLimitActive()) { + return; + } if ($this->HasActiveParent()) { if (time() - intval($this->GetBuffer('KeepAlive')) > 60 /* Seconds */) { $this->SendDebug('KeepAlive', 'Failed. Reregistering...', 0); @@ -205,6 +226,10 @@ public function ResetRateLimit() $this->updateRateLimitNotice(); $this->SetStatus(IS_ACTIVE); $this->SetTimerInterval('RateLimit', 0); + + // Limit is over - resume the event stream exactly once. + $this->SetTimerInterval('Reconnect', 0); + $this->RegisterServerEvents(); } public function GetConfigurationForm() @@ -259,6 +284,31 @@ protected function ProcessOAuthData() } } + /** + * Detects a 429 rate-limit response carried by the SSE stream and activates + * the rate limit. Envelope-agnostic: matches on the raw payload because the + * IO wraps the body in different ways. Returns true if a 429 was handled. + */ + private function applyRateLimitFromStream(string $JSONString): bool + { + if (!preg_match('/"key"\s*:\s*"429"/', $JSONString)) { + return false; + } + + $retryAfter = preg_match('/remaining period of (\d+) seconds/', $JSONString, $m) ? (int) $m[1] : 60; + + $type = null; + if (strpos($JSONString, 'in 1 day') !== false) { + $type = 'day'; + } elseif (strpos($JSONString, 'in 1 minute') !== false) { + $type = 'minute'; + } + + $this->SendDebug('ReceiveRateLimit', sprintf('429 from event stream, blocking for %ds', $retryAfter), 0); + $this->applyRateLimit($retryAfter, $type); + return true; + } + private function resetRetries() { $this->SetTimerInterval('Reconnect', 0); @@ -527,32 +577,41 @@ private function handleHttpErrors($code, $responseHeader) //Too Many Requests case 429: $head = $this->parseResponseHeaders($responseHeader); - $retryAfter = $this->getRateLimitDelay($responseHeader); - $nextRun = time() + $retryAfter; - $this->WriteAttributeInteger('RateLimitUntil', $nextRun); - $this->SetTimerInterval('RateLimit', $retryAfter * 1000); - - $this->WriteAttributeString( - 'RateError', - isset($head['rate-limit-type']) ? - sprintf( - $this->Translate( - 'The rate limit of %s was reached. Requests are blocked until %s.' - ), - $head['rate-limit-type'] == 'day' ? - $this->Translate('1000 calls in 1 day') : $this->Translate('50 calls in 1 minute'), - date('d.m.Y H:i:s', $nextRun), - ) : sprintf($this->Translate('A rate limit was reached. Requests are blocked until %s.'), date('d.m.Y H:i:s', $nextRun)) - ); - $this->updateRateLimitNotice(); - if ($this->HasActiveParent() && $this->GetStatus() != IS_ACTIVE) { - $this->SetStatus(IS_ACTIVE); - } + $this->applyRateLimit($this->getRateLimitDelay($responseHeader), $head['rate-limit-type'] ?? null); return; } } + /** + * Activates the rate-limit state shared by the REST and the SSE path: + * remembers the block end, schedules the reset timer and updates the notice. + */ + private function applyRateLimit(int $retryAfter, ?string $type): void + { + $retryAfter = max(1, $retryAfter); + $nextRun = time() + $retryAfter; + $this->WriteAttributeInteger('RateLimitUntil', $nextRun); + $this->SetTimerInterval('RateLimit', $retryAfter * 1000); + + $this->WriteAttributeString( + 'RateError', + $type !== null ? + sprintf( + $this->Translate( + 'The rate limit of %s was reached. Requests are blocked until %s.' + ), + $type == 'day' ? + $this->Translate('1000 calls in 1 day') : $this->Translate('50 calls in 1 minute'), + date('d.m.Y H:i:s', $nextRun), + ) : sprintf($this->Translate('A rate limit was reached. Requests are blocked until %s.'), date('d.m.Y H:i:s', $nextRun)) + ); + $this->updateRateLimitNotice(); + if ($this->HasActiveParent() && $this->GetStatus() != IS_ACTIVE) { + $this->SetStatus(IS_ACTIVE); + } + } + private function getData($endpoint) { $opts = [ diff --git a/Home Connect Device/module.php b/Home Connect Device/module.php index ffd1f40..844d96d 100644 --- a/Home Connect Device/module.php +++ b/Home Connect Device/module.php @@ -71,6 +71,7 @@ public function Create() $this->RegisterAttributeString('Settings', '[]'); $this->RegisterAttributeString('OptionKeys', '[]'); $this->RegisterAttributeString('InitializationSignature', ''); + $this->RegisterAttributeBoolean('Initialized', false); //Common States //States @@ -157,7 +158,7 @@ public function MessageSink($Timestamp, $SenderID, $MessageID, $Data) return; case FM_DISCONNECT: - $this->SetStatus(IS_INACTIVE); + $this->setInstanceStatus(IS_INACTIVE); return; } } @@ -385,14 +386,11 @@ public function RequestAction($Ident, $Value) public function InitializeDevice() { - // Write signature before API calls to prevent retry loops when initialization fails - // (e.g. device offline or rate limit hit mid-way through setupSettings) - $this->WriteAttributeString('InitializationSignature', $this->getInitializationSignature()); + $this->WriteAttributeBoolean('Initialized', false); if ($this->createStates()) { $this->setupSettings(); if ($this->createPrograms()) { $this->ensureControlVariable(2); - //If the device is inactive, we cannot retrieve information about the current selected Program if (@IPS_GetObjectIDByIdent('OperationState', $this->InstanceID) && ($this->GetValue('OperationState') == 'BSH.Common.EnumType.OperationState.Ready')) { $this->updateOptionValues($this->getSelectedProgram()); } @@ -400,6 +398,8 @@ public function InitializeDevice() $this->createEventProfile(); $this->MaintainVariable('Event', $this->Translate('Event'), VARIABLETYPE_STRING, 'HomeConnect.Event.' . $this->ReadPropertyString('DeviceType'), 0, true); $this->MaintainVariable('EventDescription', $this->Translate('Event Description'), VARIABLETYPE_STRING, '', 0, true); + $this->WriteAttributeString('InitializationSignature', $this->getInitializationSignature()); + $this->WriteAttributeBoolean('Initialized', true); } } @@ -439,6 +439,7 @@ public function RequestDataFromParent(string $endpoint, string $payload = '') case 'SDK.Error.UnsupportedOperation': case 'SDK.Error.NoProgramSelected': case 'SDK.Error.HomeAppliance.Connection.Initialization.Failed': + case '429': return $response; default: @@ -461,11 +462,20 @@ private function refreshDeviceState(bool $initializeDevice): void if ($initializeDevice) { $this->InitializeDevice(); } - $this->SetStatus(IS_ACTIVE); + $this->setInstanceStatus(IS_ACTIVE); return; } - $this->SetStatus(IS_INACTIVE); + $this->setInstanceStatus(IS_INACTIVE); + } + + private function setInstanceStatus(int $status): void + { + // Only update the status when it actually changes, so monitoring + // (e.g. EventControl scripts) is not triggered on every refresh. + if ($this->GetStatus() !== $status) { + $this->SetStatus($status); + } } private function needsInitialization(): bool @@ -474,7 +484,7 @@ private function needsInitialization(): bool return false; } - if (!@IPS_GetObjectIDByIdent('OperationState', $this->InstanceID)) { + if (!$this->ReadAttributeBoolean('Initialized')) { return true; } diff --git a/library.json b/library.json index 38e345c..50e375b 100644 --- a/library.json +++ b/library.json @@ -7,6 +7,6 @@ "version": "6.0" }, "version": "1.1", - "build": 11, - "date": 1781870400 + "build": 14, + "date": 1782691200 } diff --git a/tests/HomeConnectCloudTest.php b/tests/HomeConnectCloudTest.php new file mode 100644 index 0000000..7eb0107 --- /dev/null +++ b/tests/HomeConnectCloudTest.php @@ -0,0 +1,104 @@ +ConfiguratorID = IPS_CreateInstance('{CA0E667D-8F28-8DF1-2750-5CF587ECA85A}'); + + parent::setUp(); + } + + /** + * A 429 carried by the event stream must activate the shared rate-limit state, + * even though no REST call (getData/putData) was involved. + */ + public function testReceiveDataActivatesRateLimitOn429() + { + $cloud = $this->cloud(); + $cloud->ReceiveData(self::RATE_LIMIT_PAYLOAD); + + $this->assertTrue($this->invoke($cloud, 'isRateLimitActive'), 'A 429 from the stream must activate the rate limit'); + + $until = $this->invoke($cloud, 'ReadAttributeInteger', 'RateLimitUntil'); + $this->assertGreaterThan(time() + 18000, $until, 'RateLimitUntil should reflect the ~18113s retry-after'); + + $this->assertNotSame('', $this->invoke($cloud, 'ReadAttributeString', 'RateError'), 'RateError should be set'); + } + + /** + * Core of the fix: while rate limited, RegisterServerEvents must NOT reconnect + * the event stream (which would hit /events again) but defer via the Reconnect + * timer. With the old code it would fall through and try to talk to the parent IO. + */ + public function testRegisterServerEventsDefersWhileRateLimited() + { + $cloud = $this->cloud(); + $cloud->ReceiveData(self::RATE_LIMIT_PAYLOAD); + + //Must not throw and must not touch the parent IO. + $cloud->RegisterServerEvents(); + + $reconnect = $this->invoke($cloud, 'GetTimerInterval', 'Reconnect'); + $this->assertGreaterThan(0, $reconnect, 'Reconnect must be deferred until the limit expires'); + } + + /** + * The 60s keep-alive check must not trigger a reconnect while rate limited - + * otherwise it hammers /events every minute (~1440 calls/day). + */ + public function testCheckServerEventsSkipsWhileRateLimited() + { + $cloud = $this->cloud(); + $cloud->ReceiveData(self::RATE_LIMIT_PAYLOAD); + + //Would throw (parent IO has no URL/Active) if it tried to reconnect. + $cloud->CheckServerEvents(); + + $this->assertTrue($this->invoke($cloud, 'isRateLimitActive'), 'Still rate limited, no reconnect attempted'); + } + + /** + * A normal keep-alive event must still be processed (and not be mistaken for a + * rate-limit payload). + */ + public function testKeepAliveStillProcessedWhenNotLimited() + { + $cloud = $this->cloud(); + $cloud->ReceiveData('{"Event":"KEEP-ALIVE"}'); + + $this->assertFalse($this->invoke($cloud, 'isRateLimitActive'), 'A keep-alive must not activate the rate limit'); + } + + private function cloud() + { + return IPS\InstanceManager::getInstanceInterface(IPS_GetInstanceListByModuleID(self::CLOUD_GUID)[0]); + } + + private function invoke($object, string $method, ...$args) + { + $ref = new ReflectionMethod($object, $method); + $ref->setAccessible(true); + return $ref->invoke($object, ...$args); + } +} diff --git a/tests/HomeConnectFridgeFreezerTest.php b/tests/HomeConnectFridgeFreezerTest.php new file mode 100644 index 0000000..1a50665 --- /dev/null +++ b/tests/HomeConnectFridgeFreezerTest.php @@ -0,0 +1,99 @@ +ConfiguratorID = IPS_CreateInstance('{CA0E667D-8F28-8DF1-2750-5CF587ECA85A}'); + + //Reset the request counter so each test starts from a known state + HomeConnectCloud::$requestCount = 0; + + parent::setUp(); + } + + /** + * Regression test for the rate-limit retry loop. + * + * A FridgeFreezer has no OperationState in /status and /programs returns an + * UnsupportedOperation error. With the old code needsInitialization() returned + * true on every IM_CHANGESTATUS (because the OperationState variable never + * existed), so each parent status flap re-ran a full InitializeDevice() and + * burned through the API quota. This test asserts that after one successful + * initialization, repeated IM_CHANGESTATUS events trigger no further API calls. + */ + public function testNoReInitLoopWhenOperationStateMissing() + { + $fridge = IPS_CreateInstance(self::DEVICE_GUID); + $parent = IPS_GetInstance($fridge)['ConnectionID']; + IPS\InstanceManager::setStatus($parent, IS_ACTIVE); + + IPS_SetProperty($fridge, 'HaID', self::FRIDGE_HAID); + IPS_SetProperty($fridge, 'DeviceType', 'FridgeFreezer'); + IPS_ApplyChanges($fridge); + + //The first initialization ran and created variables, even though /programs failed. + $this->assertGreaterThan(0, HomeConnectCloud::$requestCount, 'Initial setup should perform API calls'); + $this->assertNotFalse(@IPS_GetObjectIDByIdent('DoorState', $fridge), 'DoorState should be created from /status'); + //A FridgeFreezer has no OperationState - this is exactly what triggered the old loop. + $this->assertFalse(@IPS_GetObjectIDByIdent('OperationState', $fridge), 'FridgeFreezer has no OperationState'); + $this->assertEquals(IS_ACTIVE, IPS_GetInstance($fridge)['InstanceStatus']); + + //Now simulate repeated parent status flaps (as seen during reconnect / rate limiting). + HomeConnectCloud::$requestCount = 0; + $intf = IPS\InstanceManager::getInstanceInterface($fridge); + for ($i = 0; $i < 5; $i++) { + $intf->MessageSink(0, $parent, IM_CHANGESTATUS, [IS_ACTIVE]); + } + + //No further API call must happen - the device is already initialized. + $this->assertSame(0, HomeConnectCloud::$requestCount, 'Repeated IM_CHANGESTATUS must not re-initialize the device'); + } + + /** + * After a config change (HaID/DeviceType) the initialization signature changes, + * so a single re-initialization is expected - but still no endless loop. + */ + public function testReInitializesOnceAfterSignatureChange() + { + $fridge = IPS_CreateInstance(self::DEVICE_GUID); + $parent = IPS_GetInstance($fridge)['ConnectionID']; + IPS\InstanceManager::setStatus($parent, IS_ACTIVE); + + IPS_SetProperty($fridge, 'HaID', self::FRIDGE_HAID); + IPS_SetProperty($fridge, 'DeviceType', 'FridgeFreezer'); + IPS_ApplyChanges($fridge); + + $this->assertGreaterThan(0, HomeConnectCloud::$requestCount); + + //Re-applying without changes must not initialize again. + HomeConnectCloud::$requestCount = 0; + IPS_ApplyChanges($fridge); + $this->assertSame(0, HomeConnectCloud::$requestCount, 'ApplyChanges without changes must not re-initialize'); + } +} diff --git a/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/programs/response.json b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/programs/response.json new file mode 100644 index 0000000..527d1c9 --- /dev/null +++ b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/programs/response.json @@ -0,0 +1,6 @@ +{ + "error": { + "description": "Operation not supported for HomeAppliance type FridgeFreezer", + "key": "SDK.Error.UnsupportedOperation" + } +} diff --git a/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/Refrigeration.FridgeFreezer.Setting.SetpointTemperatureRefrigerator/response.json b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/Refrigeration.FridgeFreezer.Setting.SetpointTemperatureRefrigerator/response.json new file mode 100644 index 0000000..0760ede --- /dev/null +++ b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/Refrigeration.FridgeFreezer.Setting.SetpointTemperatureRefrigerator/response.json @@ -0,0 +1,15 @@ +{ + "data": { + "key": "Refrigeration.FridgeFreezer.Setting.SetpointTemperatureRefrigerator", + "name": "Kühlraumtemperatur", + "unit": "°C", + "type": "Int", + "value": 4, + "constraints": { + "min": 2, + "max": 8, + "stepsize": 1, + "access": "readWrite" + } + } +} diff --git a/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/Refrigeration.FridgeFreezer.Setting.SuperModeRefrigerator/response.json b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/Refrigeration.FridgeFreezer.Setting.SuperModeRefrigerator/response.json new file mode 100644 index 0000000..375a294 --- /dev/null +++ b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/Refrigeration.FridgeFreezer.Setting.SuperModeRefrigerator/response.json @@ -0,0 +1,11 @@ +{ + "data": { + "key": "Refrigeration.FridgeFreezer.Setting.SuperModeRefrigerator", + "name": "Super-Modus Kühlraum", + "type": "Boolean", + "value": false, + "constraints": { + "access": "readWrite" + } + } +} diff --git a/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/response.json b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/response.json new file mode 100644 index 0000000..374dfa6 --- /dev/null +++ b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/settings/response.json @@ -0,0 +1,17 @@ +{ + "data": { + "settings": [ + { + "key": "Refrigeration.FridgeFreezer.Setting.SetpointTemperatureRefrigerator", + "value": 4, + "unit": "°C", + "name": "Kühlraumtemperatur" + }, + { + "key": "Refrigeration.FridgeFreezer.Setting.SuperModeRefrigerator", + "value": false, + "name": "Super-Modus Kühlraum" + } + ] + } +} diff --git a/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/status/response.json b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/status/response.json new file mode 100644 index 0000000..7719b54 --- /dev/null +++ b/tests/homeappliances/BOSCH-KAD92HBFP-68A40E25B8F3/status/response.json @@ -0,0 +1,12 @@ +{ + "data": { + "status": [ + { + "key": "BSH.Common.Status.DoorState", + "value": "BSH.Common.EnumType.DoorState.Closed", + "name": "Tür", + "displayvalue": "Geschlossen" + } + ] + } +}