Fix detection of CPU temperature sensor support on olde FRITZ!Box models#169620
Fix detection of CPU temperature sensor support on olde FRITZ!Box models#169620
Conversation
|
Hey there @AaronDavidSchneider, @chemelli74, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent the FRITZ! integration from attempting to set up the CPU temperature sensor on older FRITZ!Box models that don’t support the (experimental/undocumented) HTTP endpoint and can raise fritzconnection exceptions (e.g., HTTP 403).
Changes:
- Extend CPU temperature sensor suitability detection to treat
FritzConnectionExceptionas “not supported”. - Extend the existing CPU temperature “not supported” test matrix to include a fritzconnection exception case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
homeassistant/components/fritz/sensor.py |
Treat fritzconnection connection exceptions as “CPU temperature not supported” during suitability probing. |
tests/components/fritz/test_sensor.py |
Add an additional parametrized test case for CPU temp support detection when fritzconnection raises. |
| try: | ||
| cpu_temp = status.get_cpu_temperatures()[0] | ||
| except RequestException, IndexError: | ||
| except RequestException, IndexError, FritzConnectionException: | ||
| _LOGGER.debug("CPU temperature not supported by the device") | ||
| return False |
There was a problem hiding this comment.
Handle CPU temperature retrieval failures consistently by also catching the same exceptions in _retrieve_cpu_temperature_state (e.g., IndexError / FritzConnectionException) that _is_suitable_cpu_temperature treats as "not supported", so an empty/unauthorized response during later updates can’t bubble up and break coordinator updates.
| @pytest.mark.parametrize( | ||
| ("side_effect", "return_values"), | ||
| [(RequestException("boom"), None), (None, [0, 0, 0]), (None, [])], | ||
| [ | ||
| (RequestException("boom"), None), | ||
| (None, [0, 0, 0]), | ||
| (None, []), | ||
| (FritzConnectionException("boom"), None), | ||
| ], |
There was a problem hiding this comment.
Add a parametrized case for the reported FritzAuthorizationError (HTTP 403) in addition to FritzConnectionException so the test directly exercises the exact failure mode described in the issue.
|
I'm not sure Are we sure there is no endpoint to get this info from ? Users reported temperature is shown in Fritzbox UI. |
I choose this wide exception on purpose as this API itself is not documented nor obversely consistent across the different FRITZ!OS versions, those it can cause also other exceptions. As this sensor is meanwhile also disabled by default, I think the chance of hitting a real connection issue exactly with this api call, but not the other previous and subsequent api calls for the other "is suitable" checks is quiet low.
Yeah, it would need much more reverse engineering efforts to check this out across all the possible FRITZ!Box model and FRITZ!OS version combinations (as said above, this API is not documented) |
Proposed change
Older models seems to not support these experimental and undocumented http api and those causes
FritzConnectionExceptionerrors likeFritzAuthorizationErrorType of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: