lubeda / EspHoMaTriXv2

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

2023.9.1: Screen ID, Progress screen, Icon with Text #94

Closed andrewjswan closed 1 year ago

andrewjswan commented 1 year ago

2023.9.1: Screen ID, Progress screen, Icon with Text

Should solve the issues: https://github.com/lubeda/EspHoMaTriXv2/issues/83 https://github.com/lubeda/EspHoMaTriXv2/issues/91 https://github.com/lubeda/EspHoMaTriXv2/issues/92

andrewjswan commented 1 year ago

The screen ID works fine:

      - service: esphome.esp_pixel_clock_icon_screen
        data:
          icon_name: "weather_{{ states('weather.current_weather') | replace('-','_') }}|weather"
...
      - service: esphome.esp_pixel_clock_icon_screen
        data:
          icon_name: "weather_{{ states('weather.forecast_weather') | replace('-','_') }}|forecast"
...
andrewjswan commented 1 year ago

Calling icon_clock and icon_date screens with icon|day works fine.

Found bugs:

andrewjswan commented 1 year ago

There is no possibility to test it yet, but in theory the bugs found are fixed.

lubeda commented 1 year ago

what about the docs or a wiki entry for the new features?

andrewjswan commented 1 year ago

Look like all fixed ...

andrewjswan commented 1 year ago

what about the docs or a wiki entry for the new features?

If you can tell me which file to make the changes in, I'll do it tomorrow.

PS: I can't promise screenshots

andrewjswan commented 1 year ago

what about the docs or a wiki entry for the new features?

Added to the readme...

lubeda commented 1 year ago

Hi @andrewjswan,

it is better to have a PR per new feature! In this PR there are so many new things, its hard to check them. But i will merge this PR to a new branch and test.

andrewjswan commented 1 year ago

it is better to have a PR per new feature!

I agree, but it happened here that I did all the changes in one go since I didn't have access to a computer and it was already difficult to break it down.

lubeda commented 1 year ago

Things I tested:

I'm still not happy the the screen identifier and the "|" i believe each parameter has to be unambiguously.

To sum it up, you pushed the functionality futher, but we must take care in clean code

andrewjswan commented 1 year ago

I'm still not happy the the screen identifier and the "|" i believe each parameter has to be unambiguously.

Totally agree. But I was trying to fit the functionality into the current model without expanding it or losing the basic functionality. I think I succeeded.

  • This feature is nice but the implementation is to obscure

It's really bad out there so far, different fonts, different metrics, and you have to fit it all into 8 points. I'm still working on the display.

  • perhaps the order of the parameters should be

You can always change :)

  • boot animation 👍especially if you set auto_clear_enabled: false in the display section.

I'm not quite sure what this is about, I'll have to read up on it.

andrewjswan commented 1 year ago

To sum it up, you pushed the functionality futher, but we must take care in clean code

Totally support, I tried to implement the concept, but I don't know C++, so I'm not doing as well as I wanted.