8000 aoilfx.py throws errors repeatedly when used in Home Assistant · Issue #46 · aiolifx/aiolifx · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

aoilfx.py throws errors repeatedly when used in Home Assistant #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ggruen opened this issue Feb 9, 2022 · 21 comments
Closed

aoilfx.py throws errors repeatedly when used in Home Assistant #46

ggruen opened this issue Feb 9, 2022 · 21 comments

Comments

@ggruen
Copy link
ggruen commented Feb 9, 2022

Hi -

I'm tracking down an issue in Home Assistant's Lifx integration (which uses aiolifx under the hood), and in some circumstances, the resp_set_multizonemultizone in aiolifx/aiolifx.py is getting an index out of bounds error at line 1084: https://github.com/frawau/aiolifx/blob/7ba0a3775e273ac878f286e9685939f8fa19da70/aiolifx/aiolifx.py#L1084

This, unfortunately, causes Home Assistant to fill the disk with error messages in the log file as fast as it can.

It seems that resp_set_multizonemultizone shouldn't be throwing an error - not sure if it needs to validate incoming data or if there's an off-by-one error or somesuch, and unfortunately my Python isn't strong enough to debug, so I'm creating this issue in the hopes that someone can add some more graceful error handling in resp_set_multizonemultizone.

Here's a stack trace from the call in Home Assistant (note the call to resp_set_multizonemultizone at the bottom.

2022-02-09 17:10:46 ERROR (MainThread) [homeassistant] Error doing job: Exception in callback _SelectorDatagramTransport._read_ready()
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/local/lib/python3.9/asyncio/selector_events.py", line 1029, in _read_ready
    self._protocol.datagram_received(data, addr)
  File "/usr/local/lib/python3.9/site-packages/aiolifx/aiolifx.py", line 180, in datagram_received
    getattr(self, setmethod)(response)
  File "/usr/local/lib/python3.9/site-packages/aiolifx/aiolifx.py", line 1084, in resp_set_multizonemultizone
    self.color_zones[i] = resp.color[i - resp.index]

Thanks!

Grant
@Djelibeybi
Copy link
Contributor

If it helps, this happens when a Beam or Z changes the number of zones it has from the amount it had at discovery.

@amelchio
Copy link
Contributor

Yes, that helps. I guess it can happen if you plug/unplug a segment while the light is already connected?

I can have a look at fixing this but it will be some days before I have the time.

@frawau
< 8000 span class="Button-content"> Copy link
Collaborator
frawau commented Feb 11, 2022

I do not have a device I could test with... Would a simple try/except work as a quick fix?

@Djelibeybi
Copy link
Contributor

I'm away for the weekend but I can test when I'm back home on Sunday evening.

frawau added a commit that referenced this issue Feb 11, 2022
@frawau
Attempting to solve issue #46 by extending color_zone if it needs to …
624e0b3 
…be. NOT TESTED. Do not have compatible device to test.
@frawau
Copy link
Collaborator
frawau commented Feb 11, 2022

re looking at it, I may need to fill the gap with something before append'ing

frawau added a commit that referenced this issue Feb 11, 2022
@frawau
Attempting to solve issue #46 take 2. Making sure the append correspo…
8a7491f 
…nd to index i. Fill interval with valid 
8000
value, abeit probably incorrect. NOT TESTED. Do not have compatible device to test.
@frawau
Copy link
Collaborator
frawau commented Feb 11, 2022

Someone please test.... Do change the number of zones dynamically to check how bad the fix is ;)

@frawau
Copy link
Collaborator
frawau commented Feb 11, 2022

BTW still need a way to shorten the color_zone list... not sure how that should be done.

@ggruen
Copy link
Author
ggruen commented Feb 11, 2022 •
edited
Loading

Looks like the try/except block should fix the immediate issue (and hopefully not uncover a larger issue). I'll try installing/testing over the weekend if I can figure out how. If there's a specific program or set of commands you'd like me to run to test, let me know, otherwise I'll try installing per the README and plug and unplug lights to see if I can break anything.

@frawau
Copy link
Collaborator
frawau commented Feb 12, 2022

To get the fixed version, you need to get this git repo and do
sudo python3 setup.py install

from within. I did the minimum testing i.e. checking that my Lifx could be reached and controlled)

@frawau
Copy link
Collaborator
frawau commented Feb 19, 2022

Any feedback on this?

@Djelibeybi Djelibeybi mentioned this issue Feb 19, 2022
Merged
@Djelibeybi
Copy link
Contributor

I just pushed the fix to the embedded copy of aiolifx I ship with my modified LIFX integration at https://github.com/Djelibeybi/ha-lifx-beta/releases/tag/v2.3-alpha -- this will allow for additional feedback.

@ggruen
Copy link
Author
ggruen commented Feb 20, 2022 •
edited
Loading

Thanks Avi. I don't have a great way to reproduce the issue since I only see it through Home Assistant. Is there a way for me to try the beta version of the Lifx integration without blowing up my HA setup (I'm running a HassOS box on a Raspberry Pi 4)?

@Djelibeybi
Copy link
Contributor
Djelibeybi commented Feb 20, 2022 •
edited
Loading

Sure, just add the repo via HACS (if you're running it) or clone the repo somewhere and copy the custom_components folder into your config location. If you add it via HACS, you'll need to click the "Show Beta releases" check box when installing.

@Djelibeybi
Copy link
Contributor

Seems to have solved it for me. I've had two Beams drop segments since making the change but without the usual error log storm.

@frawau
Copy link
Collaborator
frawau commented Feb 22, 2022

@Djelibeybi Sounds good. Any issue? Are things working when you add back segments (if that is possible that is ;) Can you fully control the device after some segments were dropped?

We shall wait a few day to get some feedback before I push this as a new release to PyPi.

@Djelibeybi
Copy link
Contributor

@frawau everything seems to work fine for me, but I agree we should wait a few more days. I tend not to control my multizone devices at the zone level, so I'm not sure if it's functional to that level.

@Djelibeybi
Copy link
Contributor

I haven't heard (or seen) any negative feedback for this change, so if you can push a new version to PyPi, I'm happy to submit the corresponding update to the LIFX integration for HA (unless someone else wants to do it).

@frawau
Copy link
Collaborator
frawau commented Mar 1, 2022

Pushed 0.7.1 to PyPi.

@Djelibeybi
Copy link
Contributor

Could you please tag 0.7.1 here on GitHub and update the changelog accordingly? The Home Assistant folks review that sort of thing for a pure dependency update.

@frawau
Copy link
Collaborator
frawau commented Mar 5, 2022

Just did. Can we close this issue now?

@Djelibeybi
Copy link
Contributor

Yep, this can be closed: home-assistant/core#67721

@frawau frawau closed this as completed Mar 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Djelibeybi @ggruen @frawau @amelchio
0