lubeda / EspHoMaTriXv2

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

[FEATURE REQUEST] Add a screen identifier (not by icon name, but unique) #83

Closed andrewjswan closed 11 months ago

andrewjswan commented 1 year ago

Feature Request

Is your feature request related to a problem?

Failure to display two screens with different meanings but the same icons, for example current weather and weather forecast. We have a limited number of icons, so sharing icons with different name is not feasible.

Describe the solution / feature you'd like

Add screen_name to icon_screen, rainbow_icon_screen services, if screen_name is specified it is used for screen identification, if not specified it takes the value of icon_name field. In the services del_screen, force_screen replace the icon_name parameter with screen_name (or leave it as it is, but search for screen by the screen_name parameter), to accurately identification the screen.

If make these changes, then it will be possible to display different data with the same icons, for example, the current weather, and the weather forecast for tomorrow.

trip5 commented 1 year ago

This might be a similar reply as I made to the other post... Try making multiple IDs that refer to the same icon. ESPHome compresses the files anyway so duplicate files would not take up much extra flash size.

It'd be an easy way to see if you can find out if the actual limit is 90 anyways.

andrewjswan commented 1 year ago

I already have 90 icons, I even had to delete some of them, because I reached the limit of 90 icons. If I add even one icon, I get an error when building the project.

andrewjswan commented 1 year ago

In my opinion, linking to the icon name is not a good option, but adding a screen name is both more flexible and logical

lubeda commented 1 year ago

In my opinion the icon name is the best way. Why should I use two different values with the same icon? How can I distinguish between the two values? I need a detailed example to judge your request.

andrewjswan commented 1 year ago

icon_name: icon that is displayed screen_name: screen identifier

There is no problem with finding the right icon and the right screen because if only icon_name is specified in the call then screen_name will take its value, if both values are specified then each will have its own value.

This will allow to display the current weather on one screen and the weather forecast on the second screen. The icons can be the same, for example sunny, but screen_name we can specify different weather and forecast. And it will allow to update the data not to delete the screen, but just update it by screen_name.

andrewjswan commented 1 year ago

Old way: (The only thing you can take out is the weather)

      - service: esphome.esp_pixel_clock_del_screen
        data:
          icon_name: weather_*
          mode: 5
      - service: esphome.esp_pixel_clock_icon_screen
        data:
          icon_name: "weather_{{ states('weather.current_weather') | replace('-','_') }}"
          text: "{{ states('sensor.temperature_outside') }}°C"
          lifetime: 320
          screen_time: 10
          default_font: false
          r: 200
          g: 200
          b: 200

New way:

      - service: esphome.esp_pixel_clock_icon_screen
        data:
          icon_name: "weather_{{ states('weather.current_weather') | replace('-','_') }}"
          screen_name: weather
          text: "{{ states('sensor.temperature_outside') }}°C"
          lifetime: 320
          screen_time: 10
          default_font: false
          r: 200
          g: 200
          b: 200
      - service: esphome.esp_pixel_clock_icon_screen
        data:
          icon_name: "weather_{{ states('weather.forecast_weather') | replace('-','_') }}"
          screen_name: forecast
          text: "{{ states('sensor.temperature_forecast') }}°C"
          lifetime: 320
          screen_time: 10
          default_font: false
          r: 200
          g: 200
          b: 200

While the old option will work because screen_name is optional, and if not specified, will take the value icon_name.

andrewjswan commented 1 year ago

Looked at the source code, you have basically done the same thing, but there is no way to set the screen identifier from HA, you use icon and icon_name, the first is the icon itself, the second is essentially the screen identifier. https://github.com/lubeda/EspHoMaTriXv2/blob/main/components/ehmtxv2/EHMTX_queue.cpp#L13-L14

You can leave the names as they are, or you can change icon_name to screen_name or to screen_id, and add icon or icon_name field to icon_screen and rainbow_icon_screen services, which will be used only for icon search when adding a screen.

  void EHMTX::icon_screen(std::string iconname, std::string screenname, std::string text, int lifetime, int screen_time, bool default_font, int r, int g, int b)
  {
    uint8_t icon = this->find_icon(iconname.c_str());

    if (icon >= this->icon_count)
    {
      ESP_LOGW(TAG, "icon %d not found => default: 0", icon);
      icon = 0;
      for (auto *t : on_icon_error_triggers_)
      {
        t->process(iconname);
      }
    }
    EHMTX_queue *screen = this->find_icon_queue_element(icon);

    screen->text = text;
    screen->endtime = this->clock->now().timestamp + lifetime * 60;
    screen->text_color = Color(r, g, b);
    screen->default_font = default_font;
    screen->mode = MODE_ICON_SCREEN;
    screen->icon_name = screenname;
    screen->icon = icon;
    screen->calc_scroll_time(text, screen_time);
    for (auto *t : on_add_screen_triggers_)
    {
      t->process(screen->icon_name, (uint8_t)screen->mode);
    }
    ESP_LOGD(TAG, "icon screen icon: %d iconname: %s screenname: %s text: %s lifetime: %d screen_time: %d", icon, iconname.c_str(), screenname.c_str(), text.c_str(), lifetime, screen_time);
    screen->status();
  }

Or

  void EHMTX::icon_screen(std::string iconname, std::string text, int lifetime, int screen_time, bool default_font, int r, int g, int b, std::string screenname = "")
  {
    ...
    if (screenname == "")
    {
      screenname = iconname;
    }
    ...
 }
andrewjswan commented 1 year ago

PS: And a small question, in this section of the source code shouldn't the analysis of the MODE_RAINBOW_ICON https://github.com/lubeda/EspHoMaTriXv2/blob/main/components/ehmtxv2/EHMTX.cpp#L939 Like:

if (((this->queue[i]->mode == MODE_ICON_SCREEN) || (this->queue[i]->mode == MODE_RAINBOW_ICON)) && (this->queue[i]->icon == icon))
andrewjswan commented 11 months ago

Hi @lubeda ! How do you look at using a screen ID anyway? You have everything ready for this, https://github.com/lubeda/EspHoMaTriXv2/issues/83#issuecomment-1727141991 you just need to allow the user to set it when calling the service (it can be optional), if it is specified then use it, if it is not specified then use the name of the icon.

PS: I have one question, why separate color assignment is used, i.e. r,g,b and not rgb array, because we can pass color as int[] array....

r: 200
g: 200
b: 200

vs

rgb: [200,200,200]

or

rgb:
  - 200
  - 200
  - 200
andrewjswan commented 11 months ago

I think I've found an elegant solution to screen identification. I suggest adding the ability to specify the screen ID in the icon name (for those who need it), like this: icon_name|screen_id

  void EHMTX::icon_screen(std::string iconname, std::string text, int lifetime, int screen_time, bool default_font, int r, int g, int b)
  {
    uint8_t icon = this->find_icon(get_icon_name(iconname).c_str());

    if (icon >= this->icon_count)
    {
      ESP_LOGW(TAG, "icon %d not found => default: 0", icon);
      icon = 0;
      for (auto *t : on_icon_error_triggers_)
      {
        t->process(get_icon_name(iconname));
      }
    }
    EHMTX_queue *screen = this->find_icon_in_queue(get_screen_id(iconname));

    screen->text = text;
    screen->endtime = this->clock->now().timestamp + lifetime * 60;
    screen->text_color = Color(r, g, b);
    screen->default_font = default_font;
    screen->mode = MODE_ICON_SCREEN;
    screen->icon_name = get_screen_id(iconname);
    screen->icon = icon;
    screen->calc_scroll_time(text, screen_time);
    for (auto *t : on_add_screen_triggers_)
    {
      t->process(screen->icon_name, (uint8_t)screen->mode);
    }
    ESP_LOGD(TAG, "icon screen icon: %d iconname: %s screenname: %s text: %s lifetime: %d screen_time: %d", icon, get_icon_name(iconname).c_str(), get_screen_id(iconname).c_str(), text.c_str(), lifetime, screen_time);
    screen->status();
  }

Functions returning the Icon name or Screen ID are like this:

std::string get_icon_name(std::string iconname)
{
    std::stringstream stream(iconname);
    std::string icon;
    std::vector<std::string> tokens;

    while (std::getline(stream, icon, '|'))
    {
        if (!icon.empty())
        {
            tokens.push_back(icon);
        }
    }

    return (tokens.size() > 0) ? tokens[0] : "";
}

std::string get_screen_id(std::string iconname)
{
    std::stringstream stream(iconname);
    std::string screen_id;
    std::vector<std::string> tokens;

    while (std::getline(stream, screen_id, '|'))
    {
        if (!screen_id.empty())
        {
            tokens.push_back(screen_id);
        }
    }

    return (tokens.size() > 1) ? tokens[1] : (tokens.size() > 0) ? tokens[0] : "";
}

Tested it on different variants, works like clockwork: https://onlinegdb.com/kTm8C-UW4