ph1p / ikea-led-obegraensad

ESP32/Arduino hack for the ikea OBEGRÄNSAD led wall lamp
MIT License
578 stars 78 forks source link

Character Font, Scrolling Text, API, Alternative Clock with Second Pixel #69

Closed kohlsalem closed 5 months ago

kohlsalem commented 7 months ago

Hi,

i implemented the followig features:

to my assesment this changes should not change/break anything existing...

Best Michael

jekkos commented 7 months ago

Thanks! Very nice. This was a feature I was planning to add myself. Also the REST endpoint to send messages can now be used by home assistant, so the panel can display notifications.

kohlsalem commented 7 months ago

I added propper message handling and printing of graphs,. Obviously i am a little bit "under knowledged", i was very surprised to find my commits automatically added in this PR, i planned to have it in a seperate one.

However, @ph1p, has this monster now have a chance to get mergend?

ph1p commented 7 months ago

Hi @kohlsalem :) First of all, thank you so much for this PR! And it definitely gets a chance to be merged! I think display text is one of the biggest missing features. I also flashed it on my ESP32 and it works fine, except for long texts. My ESP hangs when sending texts that are too long.

Great work!

kohlsalem commented 7 months ago

Hmm, which version? in the vesion before my very last commit i found the same problem- aparrently it is not good to spend to much time in a http handler. However, in the most recent commit i fixed that....

Where is your upper limit? If it still crashes that would mean i can not blockout the loop for a longer time. Solvable but... ugly.

kohlsalem commented 7 months ago

thank you so much for this PR!

"Danichfüa!" Thanks for that project. Nicely crafted! It was looking so obviously ahead the the hacked scetch from C'T, that i decided to throw out the 8266 and soildered a esp32...

Just to see that a font is missing :-D i added the scrolling already to the 8266 code, so that part was c&p. Converting the font was a but more tricky and ended up in a monster-excel-sheet.

I added some fixes and a chapter to the readme. from my POV it should me somewhat ok...

kohlsalem commented 7 months ago

Closes https://github.com/ph1p/ikea-led-obegraensad/issues/9

kohlsalem commented 7 months ago

Thanks for proposals - yes, i have obviously overseen dev clutter.

@ph1p, delay is clear, that was already on my todo list. And as you said, should'nt be a big deal.

What would the loop flag be for? If you do not pass any repeat, the message shows only once.

I did think about some infinite looping (only show messages) undtil you press the button. Was that your idea?

kohlsalem commented 7 months ago
kohlsalem commented 7 months ago

and some bug fixes and minor improvements

kohlsalem commented 6 months ago

@ph1p Hi Phil, normally i should have no big pressure with the merge, but since i messed with the propper use of branches, i brought myself a bit into a situation i cant resolveby myself. trying to get the recently meged PRs i messed up beyond my abilities. Is there a way to merge this PR as it is, or shall i better startover with a new fork and try to recreate the PR?

MauiKano commented 6 months ago

I did add the changes from kohlsalem to my fork under https://github.com/MauiKano/ikea-led-obegraensad Works well for me an I do like the messages a lot. I used it for displaying a boot message in the main.cpp and started writing my own plugin FiveLetterWords. This randomly shows a word out of a large list of words from https://github.com/charlesreid1/five-letter-words . More could be done with that plugin. Oh yes, I modified the simple clock plugin and added a second indicator. I pixel that wanders around the edge of the frame which happens to have 60 pixels in total :-)

ph1p commented 6 months ago

Hi! Could you resolve the conflicts? I'll merge this afterwards. Thank you :)

MauiKano commented 6 months ago

you catch me on the wrong foot. which conflict ?

kohlsalem commented 6 months ago

Hmm. It is emparrassing, but cant resolve the conflicts (unless someone is willing to help me with the version control mess i probably created).

MauiKano commented 6 months ago

can you please be more specific ? I was able to pull your code into my fork and all compiles and runs well.

kohlsalem commented 6 months ago

lease be more specific ? I was able to pull your code into my fork and all compiles and runs well.

Well, despite my decades in software development if am maximum stupid in work with git. since i could not understand how to use it directly in vscode propperly, use it via the githup desktop. I tried quire some htings, but did not suceed to resolve the conflicts, Not logically (the seem minimal) but technically in github. If you could hint me via teams (michael@kohlsalem.com) would be awsome, as i said, i am a more than a little list in the branches...

ph1p commented 6 months ago

I'll fix the commit history and resolve the conflicts :)

kohlsalem commented 6 months ago

I'll fix the commit history and resolve the conflicts :)

you are my hero. I promise to do proper branches for my next feature and hope i can do better with this...

ph1p commented 6 months ago

I am now done with the cleanup. Please delete your branch locally and checkout your repository again as I have force pushed. I have tested the feature and it is not working as it should. The repeat flag does nothing. When the message has been sent, the previous state appears after the message and then the message appears again and again, seemingly randomly. Is this intentional?

kohlsalem commented 6 months ago

Thanks!

Well, i do the repeat every minute, may be that detail is not emphasized in the readme. So, every next full minute all message with a repeat flag gets repeated.

kohlsalem commented 6 months ago

shall i checkout ph1p:main? because there i cant find it yet.

ph1p commented 6 months ago

Ah I see. So the repeat is after 1 minute. This is a little bit confusion. It would be great to set this with delay and add a duration flag to determine how long it should take to see the text. What do you think?

Just checkout your repository:

git clone git@github.com:kohlsalem/ikea-led-obegraensad.git
# or 
git clone https://github.com/kohlsalem/ikea-led-obegraensad.git
kohlsalem commented 6 months ago

Hmm. I get your point. But Hmm.

The usecase i had in mind ist, that i have a default view. Clock, weather, game of live, whatever. And then some Infomessage shows.

Features i imagine:

I like the idea to show the messages at the full minute, becaus i jus know when it will be shown again. If i set the duration it will become really random.

I could imagine a "urgent" flag, which shows the Message continuously until button pressed?

kohlsalem commented 6 months ago

@ph1p so, any thoughts on my proposal? Actually, since the PC is anyway alreasy to big for my preference, i would be somehow happy if you could merege it before i add new features...@

c64emulator commented 5 months ago

Der Messagetext sollte zur Sicherheit auf gültige Zeichen überprüft werden, bevor er ausgegeben wird: momentan bootet der ESP neu, wenn Umlaute (ä,ö,ü) im Messagetext enthalten sind. Anregung: zumindest die deutschen Sonderzeichen/Umlaute würde ich noch in die Tabelle in 'signs.cpp' einarbeiten.

kohlsalem commented 5 months ago

Autsch. Das tut echt weh. Super peinlich. Nicht validierter User-Input als Lookup in einem Array. Immerhin bin ich überrascht, das der ESP mit einem Speicherschutz daher kommt. ich hätte mit Random Pixel für die Umlaute gerechnet. Wird gefixt!

c64emulator commented 5 months ago

Ich denke, es ist sinnvoll, aus diesem PR mehrere zu machen. Das Plugin mit dem Zeichensatz sollte abgetrennt werden. Die Nutzung des Zeichensatzes könnte lizenzrechtlich problematisch sein. Als Plan-B könnte auch ein anderer Zeichensatz verwendet werden (z.B. https://github.com/micropython/micropython/blob/master/extmod/font_petme128_8x8.h)

c64emulator commented 5 months ago

Noch eine Anregung für den Message-Teil: es sind ja noch ein paar Pins am ESP frei, ein Piezo-Buzzer könnte angeschlossen werden. Dann könnte bei einer Message auch ein Beep (oder sogar eine Melodie?) ausgegeben werden.

kohlsalem commented 5 months ago

Ich denke, es ist sinnvoll, aus diesem PR mehrere zu machen. Das Plugin mit dem Zeichensatz sollte abgetrennt werden. Ja, ich meine auch der PR sollte (in einner Bugfreien Version) erstmal reinmigriert werden. Wird sonst gefühlt einfach zu groß.

Zeichensätze wechselbar wäre z.B. definitiv ein separates Thema.

c64emulator commented 5 months ago

Noch eine Anmerkung zum Messageteil: Du nutzt ja momentan den 6x7-Font. Du rechnest damit auch in 'Screen.scrollText' (hardcodiert...). Zwischen den einzelnen Zeichen sollte aber mindestens ein Spalte Abstand sein, damit sie besser lesbar sind, wenn sie über das Panel scrollen.

kohlsalem commented 5 months ago

Hast Du das mit der Spalte mal ausprobiert? ich hab und hatte es für sch*** ähh, schlecht befunden...

kohlsalem commented 5 months ago

Ansonsten, für beliebige fonts /umschaltbare fonts müsste man das ganze irgendwie anders aufziehen. Da müssten die Maße dann aussen um das Array nochmal festgeschrieben werden. bzw, Es bräuchte ein Array von Fonts mit Metadaten(Größe, Namen, ...) den Fontdetails, ...

kohlsalem commented 5 months ago

and still trusting that it is worth to be merged ;-)

kohlsalem commented 5 months ago

so, and as a (hopefully last) feature to this: While Messges are pending for display, an indicator is flashin in upper left corner

ph1p commented 5 months ago

Thank you! :) I'll take a look at it

kohlsalem commented 5 months ago

Merci!

thunder-62 commented 5 months ago

aehm... apologies for the spoiler... Hopefully, I made made a mistake and someone can help me to get it corrected... When I send "http://192.168.20.59/message?text=ABC" in a GET request, my Display shows "789" seems like the index to an ASCII Table is somehow incorrect...

kohlsalem commented 5 months ago

Hmm, is it literally translating ABC to 789, or is that an example? I'm pretty sure it worked for ordinary letters.

i assume the call comes with bad coding. could you uncomment line 141 in messages.cpp? for(int i = 0;i<text.size();i++)Screen.scrollText(std::to_string(text[i])); That should print the char code which was recieved....

thunder-62 commented 5 months ago

Hi kohlsalem, it is literally ABC-->789. first I tried using just Chrome then I installed the restman chrome Extension and sent it with a pure GET request. same result.

As soon as I am back, I will uncomment the line and tell you about the result... looking at the code: it writes the chars to serial right?

Cheers, Uwe

kohlsalem commented 5 months ago

Hi Uwe,

I'll check again today, anyway strange.

I cant see, what "Last Second" Change should have caused this, since this seems to be a obious Bug :-/

Best Michael

kohlsalem commented 5 months ago

Ok. I found the issue, without testing it. It was a last minute change - or the missing ability to use Copy&Past Properly. By making changes on the fonts for digits and creating a new "font" with bold digits, i apparently pasted them at the origin. And tested only with digits sinceafter - which works fine.

In file src/signs.cpp please remove line 86-95.

THe index numbers in the comments at the end should continuosly count up.

thunder-62 commented 5 months ago

yessss... removing the lines was the fix! Thank you!

kohlsalem commented 5 months ago

@ph1p The duplication did not happen while i copied, apparently it is a glitch during your reformatting.

I do not understand, why you did reformat it? Yes, the source code is smaller, but after compilation... byte is byte....

The big benifit of thebin format (and the reason why i used it) you can see and modify characters directly in the code. That is lost on hex....