lubeda / EspHoMaTriXv2

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

scroll_small_text: false does not work #47

Closed andrew-codechimp closed 1 year ago

andrew-codechimp commented 1 year ago

Bug report

Describe the bug

The flag scroll_small_text does not work correctly. Setting it to false small text still scrolls

Additional information

I attempted to fix it as did @joncar, see links below but neither quite get's it right

https://github.com/andrew-codechimp/EspHoMaTriXv2/commit/bdf69aa729a1d1a2e3ab9fe948002dd730f2b987 https://github.com/joncar/EspHoMaTriXv2/tree/user/joncar/01-fix-scroll

To Reproduce

Steps to reproduce the behavior: Set scroll_small_text: false and add a screen with small text

Expected behavior

A clear and concise description of what you expected to happen. Small text will not scroll when set to false

Configuration

(optional) The YAML you used in epshome without any password

Screenshots

If applicable, add screenshots to help explain your problem.

Logs

(optional) Add relevant logs which could help tackle the problem.

Services calls

(optional) The YAML of your service calls
joncar commented 1 year ago

I believe my fix is correct. Did you try it and it didn't work?

andrew-codechimp commented 1 year ago

I tried it but it's inverted, setting scroll_small_text: false still scrolled small text, you have to set it to true not to scroll.

joncar commented 1 year ago

I don't see how. Strange. I just tested scroll_small_text: true, scroll_small_text: false, and with scroll_small_text unspecified and all behaved how I expected.

joncar commented 1 year ago

Any chance your edit to EHMTX_queue.cpp was still in your esphome code when you tested? That would invert the flag.

andrew-codechimp commented 1 year ago

Yes, I did leave my edit in EHMT_queue.cpp, which I didn't think I did, so I'm an idiot :) Blame it on early morning tinkering.

Are you going to submit a PR with your fix?

joncar commented 1 year ago

Yup, I'll submit the PR to fix this issue.

lubeda commented 1 year ago

Hi, i merged it to the development branch and it works fine. Now i am searching a way to merge it also to the main branch, so everybody can use it.

lubeda commented 1 year ago

merged in main. Thanx for the support!