gickowtf / pixoo-homeassistant

Home Assistant Integration for Divoom Pixoo 64
MIT License
112 stars 12 forks source link

[Feature] New update loop #54

Closed Mrredstone5230 closed 5 months ago

Mrredstone5230 commented 5 months ago

I wanted to do this for a while now, but finally I think I got it. This now uses an task/coroutine that will handle the page updating instead of using HA's events. This is great, since before, after a notification, it wouldn't respect at the letter the duration. Also, this now allows to have page duration for pages in the config!!! (I was thinking of putting it in another PR. I think it would be cleaner like that, even though it's only one line. Would close #43)

At the same time, I optimized the sensor's constructor and removed the _async_update method, which was already not really used.

Hope you like it!

gickowtf commented 5 months ago

Also, this now allows to have page duration for pages in the config!!! (I was thinking of putting it in another PR. I think it would be cleaner like that, even though it's only one line. Would close https://github.com/gickowtf/pixoo-homeassistant/discussions/43)

I think I misunderstood, but the update only applies to the automation and not to the pages config

Mrredstone5230 commented 5 months ago

opposite. This PR (and my idea for another one) only applies for config usage.

gickowtf commented 5 months ago

line 124

            if is_enabled:
                duration = self.page.get('duration', self._scan_interval.total_seconds())
                self.schedule_update_ha_state()
                await self.hass.async_add_executor_job(self._render_page, self.page)
                await self.async_schedule_next_page(duration)

quick and dirty that is my thought

Mrredstone5230 commented 5 months ago

line 124

            if is_enabled:
                duration = self.page.get('duration', self._scan_interval.total_seconds())
                self.schedule_update_ha_state()
                await self.hass.async_add_executor_job(self._render_page, self.page)
                await self.async_schedule_next_page(duration)

quick and dirty that is my thought

Yeah that's what I was thinking about too. I was thinking of putting it in another PR, but I sure can put it here..

gickowtf commented 5 months ago

okay sorry i think i just misunderstood you or thought that you were referring to the part where i define the pages as config. but i also have to say that the holidays are very exhausting, unlike expected... my wife and my daughter have a cold -.- But much more important is that we understand each other now. That's how it is in just one PR for once. But I'm also of the opinion that the PR belongs together.