home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
71.16k stars 29.84k forks source link

Russound RIO - Zones become unresponsive (core-2024.8.1) #124060

Closed brettcp closed 3 days ago

brettcp commented 1 month ago

The problem

Yesterday I noticed all 8 of my zones within russound_rio were unresponsive. HA's log shows an "Unhandled exception in IO loop" which I've seen in previous builds. This was previously addressed here: https://github.com/home-assistant/core/issues/68332 but seems to not have made it into the current build.

After this occurs, a restart of HA does not resolve it.. russound_rio fails to start until I implement the following fix:

Replacing line 93: s = str(res, "utf-8").strip()

with: s = str(res, 'utf-8', errors='replace').strip()

...appears to resolve the issue and russound_rio starts up and is again working as expected.

What version of Home Assistant Core has the issue?

core-2024.8.1

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

russound_rio

Link to integration documentation on our website

https://www.home-assistant.io/integrations/russound_rio/

Diagnostics information

Logger: aiorussound Source: /usr/local/lib/python3.12/site-packages/aiorussound/rio.py:153 First occurred: August 15, 2024 at 2:58:13 PM (11767 occurrences) Last logged: 7:27:14 AM

Unhandled exception in IO loop Traceback (most recent call last): File "/usr/local/lib/python3.12/site-packages/aiorussound/rio.py", line 121, in _ioloop self._process_response(response) File "/usr/local/lib/python3.12/site-packages/aiorussound/rio.py", line 77, in _process_response s = str(res, "utf-8").strip() ^^^^^^^^^^^^^^^^^ UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe9 in position 21: invalid continuation byte

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

Replacing line 93: s = str(res, "utf-8").strip()

with: s = str(res, 'utf-8', errors='replace').strip()

...appears to resolve the issue.

home-assistant[bot] commented 1 month ago

Hey there @noahhusby, mind taking a look at this issue as it has been labeled with an integration (russound_rio) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `russound_rio` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign russound_rio` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


russound_rio documentation russound_rio source (message by IssueLinks)

noahhusby commented 1 month ago

Are you connected directly to the controller with ethernet, or are you using an ethernet to serial adapter? Also which controller do you have?

brettcp commented 1 month ago

Are you connected directly to the controller with ethernet, or are you using an ethernet to serial adapter? Also which controller do you have?

Connected via ethernet (no serial adapter) via my LAN. Controller is an MCA-88 on firmware 04.07.00.

noahhusby commented 1 month ago

Got it.

Just wanted to check since sometimes those IP2SR adapters can inject additional characters that mess with the library. Do you have any additional context regarding when the error first came up? (It would help to have Russound RIO's debug log enabled). I'd rather find the root cause versus just silencing the exception.

brettcp commented 1 month ago

Got it.

Just wanted to check since sometimes those IP2SR adapters can inject additional characters that mess with the library. Do you have any additional context regarding when the error first came up? (It would help to have Russound RIO's debug log enabled). I'd rather find the root cause versus just silencing the exception.

I can't think of anything in particular that would have triggered it.. It didn't seem to occur right after a zone control command, etc.

I'll revert the "fix" then try to get it back to a working state, then enable the debug log until the issue occurs again.

Thank you!

brettcp commented 2 weeks ago

Got it. Just wanted to check since sometimes those IP2SR adapters can inject additional characters that mess with the library. Do you have any additional context regarding when the error first came up? (It would help to have Russound RIO's debug log enabled). I'd rather find the root cause versus just silencing the exception.

I can't think of anything in particular that would have triggered it.. It didn't seem to occur right after a zone control command, etc.

I'll revert the "fix" then try to get it back to a working state, then enable the debug log until the issue occurs again.

Thank you!

Shortly after my last post and your reply, I remembered I do actually have an IP2SR adapter on my MCA-88 which I use for a 3rd party app that sends metadata to the keypads. I unplugged it for the last week and sure enough I haven't had this crash/exception occur so I think you're right about it being the cause of this issue.

I just plugged it back in and enabled debug logging again (without applying the line 93 "fix"), will post the debug logs the next time it occurs.

brettcp commented 2 weeks ago

This error occurred this morning, I've attached the debug log here. Thanks! home-assistant_russound_rio_2024-08-26T16-13-34.634Z.log

brettcp commented 1 week ago

Is there anything else I can do to assist with this? For now I just applied the temporary "fix" (which from what I now understand is just silencing the exception). If there's anything else I can provide that may be helpful, please let me know.

noahhusby commented 1 week ago

Hi Brett,

I'm working on re-writing the IO loop on the library side which will mitigate this issue entirely. I'll leave this issue open for comments until the new version is patched.

noahhusby commented 1 week ago

After doing some more digging, I believe I've narrowed down the issue. It seems that the issue is apparent when you are listening to a stream or radio which includes author, song, album, and playlist metadata. I suspect that one of those fields contains a latin character (like é) and the Russound box is encoding the character using the iso-8859-1 charset instead of UTF-8. I'm planning on mapping characters from the first charset to UTF-8 to properly resolve the issue. the errors='replace' will cause the malformed characters to disappear rather than being encoded properly.

brettcp commented 1 week ago

After doing some more digging, I believe I've narrowed down the issue. It seems that the issue is apparent when you are listening to a stream or radio which includes author, song, album, and playlist metadata. I suspect that one of those fields contains a latin character (like é) and the Russound box is encoding the character using the iso-8859-1 charset instead of UTF-8. I'm planning on mapping characters from the first charset to UTF-8 to properly resolve the issue. the errors='replace' will cause the malformed characters to disappear rather than being encoded properly.

Great, thank you so much for all of your work on this!

noahhusby commented 1 week ago

The update to aiorussound >v3.0.0 should resolve this issue while supporting those latin characters. The PR is currently open and pending review. https://github.com/home-assistant/core/pull/125285

I'd appreciate if you gave the following line a try and see if it performs properly over the next week:

s = res.decode(encoding="iso-8859-1").encode(encoding="utf-8").decode(encoding="utf-8").strip()

The code is a bit cleaner in the new version, but this should allow you to test the functionality in the older version of the library.

brettcp commented 1 week ago

Thank you! I made the change with the line above and will test this week.

noahhusby commented 1 week ago

The change was included in release 2024.9.1. Can you upgrade and see if the issue is resolved?

noahhusby commented 3 days ago

Has the issue resurfaced at all? If not, I'll close the issue.

brettcp commented 3 days ago

Has the issue resurfaced at all? If not, I'll close the issue.

The issue has not resurfaced. I haven't upgraded to 2024.9.1 but I've been using the code from your post last week and have not had the issue occur.

Thanks so much for all of your contribution to this project!

noahhusby commented 3 days ago

Sounds good, I'm going to close this then.

If the issue resurfaces, then feel-free to open a new issue.

noahhusby commented 3 days ago

@home-assistant close