lubeda / EspHoMaTriXv2

A simple DIY status display with a 8x32 RGB LED matrix, implemented with esphome.io and Home Assistant.
MIT License
302 stars 30 forks source link

[FEATURE REQUEST] Allow duplicate icon_screen #242

Open simcop2387 opened 4 months ago

simcop2387 commented 4 months ago

Feature Request

Is your feature request related to a problem?

A bit less a request and more an idea and wanting any comments on my idea for a solution that I'm going to implement in a fork.

I've got some weather screens (Forecasts, today, tomorrow, day after tomorrow) that I want to share icons on, but with the current behavior it doesn't work without duplicating icons.

I.e. If two days have an icon of "weather_cloudy" then they overwrite each other, only showing the last one sent to the screen.

I'm currently working around this by duplicating the icons but with different IDs, but that takes up a lot of flash space of course (3x what it should).

Describe the solution / feature you'd like

What I'm going to do is alter the icon_screen() function a little bit to add a new parameter "skip_duplicate_check" as a boolean, and then use that new parameter to skip the search and instead just add it as a new screen entirely. This would mean that removing the old screens must be done by the user, and so should be documented that way, but it will allow for screens like that to share icons without wasting space.

Describe alternatives you've considered

Can't really see any other alternatives other than duplicating icons but that adds up fast for things like weather when you've got 12 icons or so.

I do wonder if there should be some additional identifier (on larger systems i'd maybe use a uuid or something) to search for screens to replace duplicate screens but i think that might be significantly more invasive of a change. Maybe adding a "name" parameter to every screen being added and using that instead?

Additional context

This would also work for other use cases too.

trip5 commented 4 months ago

I was going to write:

Are you 100% certain it takes up more flash space to use duplicate icons? That actually kind of surprises me.

I'm pretty sure things like fonts and icons are compressed using a gzip-like algorithm as part of the bin compilation. Compressed segments would only use a few extra bytes in the case of duplicate data... so 22 extra icons should only be at most a hundred bytes or so.

Instead I ran a quick test myself. 29 icons totaling less than 13KB in actual file sizes duplicated and the flash files were 90KB larger.

Indeed, I'm surprised. Duplicate fonts are compressed much more efficiently. But that might be Pillow at work behind the scene and (I don't understand the code very well) but it looks like EHMTXv2 is actually adding graphics in a fairly inefficient way.

How good are you at this code, it might be easier to implement a kind of pointer / place-holder? Ie. You still specify things the way you're using it (using screens and ids) but a reuse of the same icon can be specified like this...? Would that work?

    - id: weather_cloudy
      file: icons/weather_cloudy.gif
    - id: weather_cloudy_2
      file: *weather_cloudy*

Just spitballing here. I don't like the idea of adding further complication to how screens are added/removed using Home Assistant... re-use of icons should just be made more efficient.

simcop2387 commented 4 months ago

I think my particular case is seeing it more because i'm using some heavily animated ones from LaMetric for the weather because they look nice. For those it should be relatively simple to recognize the duplicates and handle things but I haven't looked at how the icon data really gets built yet. I've got some other ulterior motives for thinking about handling "duplicate" screens because I want to also eventually extend that to text screens and others so that I don't have to have HA constantly changing things in and out to get the same effect. I think ideally having both things work would be the best way since then duplicated icons and more flexible control over screens themselves would be possible.

andrewjswan commented 4 months ago

I've got some weather screens (Forecasts, today, tomorrow, day after tomorrow) that I want to share icons on, but with the current behavior it doesn't work without duplicating icons.

This is already done, you can add multiple identical screens without duplicating icons using the screen ID. https://github.com/lubeda/EspHoMaTriXv2/tree/latest?tab=readme-ov-file#screen_id