Feature/rest api (advanced and cleaned)

[gelse]

Based on the work of @kohlsalem, but slightly improved with the following topics:

• added getStatus endpoint
• added getMetadata endpoint, which includes the list of available plugins
• switched the set.... endpoints to PATCH instead of GET
• added (separate) getData endpoint, which just returns the current displayed data
• added a bit of errorhandling
• moved method implementation to messages.h, to make it same as the existing /message endpoint
• removed arbitrary "OK" message from Status 200 return codes

[kohlsalem]

May be im not firm enought to gh, but wouldnt it hae been smarter to let you join on PR #98? I guess now i should cose that, right?

After brief reviev: Since the handlers have nothing at all to do with message handling, i would not want to see them there nor would have ever serched them there. Having them distributed all over the place is even not so nice and comes with a include dependency hell (ok, a lot of).

I think it is way better to keep them with the webserver, as it was done with the web socket implementations.

I would be totally ok if you move the message handlers as well out of message.cpp...

[gelse]

May be im not firm enought to gh, but wouldnt it hae been smarter to let you join on PR #98? I guess now i should cose that, right?

not sure what "May be im not firm enought to gh" means, tbh ... anyway, you mean PR #99 i guess, #98 is the issue. but i know what you mean. Well yeah - propably you are right, but i did not know how i can contribute to your branch - so i did the best i could think of, created a fork, a branch, merged your branch to mine and built on top of it. Only afterwards i saw that you already also did implement the getStatus, i guess we really just missed each other by short time.

After brief reviev: Since the handlers have nothing at all to do with message handling, i would not want to see them there nor would have ever serched them there. Having them distributed all over the place is even not so nice and comes with a include dependency hell (ok, a lot of).

I think it is way better to keep them with the webserver, as it was done with the web socket implementations.

I half agree and half disagree. You are right, the rest of the message file does have nothing to do with handling web requests and all those handlers belong somewhere else. But definitely not in the asyncwebserver, because the purpose of that class is to provide the server and distribute the requests - NOT to handle them in detail. I'll take care of that, give me a bit.

I would be totally ok if you move the message handlers as well out of message.cpp...

I'll look into that as well.

update: @kohlsalem updated my branch - extracted the handlers to separate file, including the message handlers. And i think we are still far away from dependency hell.

[kohlsalem]

Dependency hell was if i put the handlers where they beloned to (Screen, Plugin Manager, ...)

[ph1p]

Thank you both! I only have one thing. There are often comments that offer no added value. I would suggest identifying and deleting them.

I mean something like this:

// Send a response to the client
request->send(200, "text/plain", "Message received");

// Call the add function with the extracted parameters
Messages.remove(id);

// Extract the 'value' parameter from the request
int value = request->arg("value").toInt();

The code itself explains what happens here.

[kohlsalem]

Fair point. I never wrote them, they are a gift of "Check and comment the following code" request to chatgpt.

@gelse, it least i take no offence from getting them removed, if anyone woulf have cared :-D

[jekkos]

Thanks for the work here! I'm using this currently with home assistant, the LED matrix now shows the artist + track that my internet radio is playing, which beside the clock animation is pretty cool. Also the brightness is reduced once the sun goes down.

For those interested here the HA config for the internet radio

in configuration.yaml

  shell_command:
     show_playing: curl http://obegransad.lan/message?text={{ state_attr('media_player.heritage_2', 'media_artist') | urlencode }}%20-%20{{ state_attr('media_player.heritage_2', 'media_title' ) | urlencode }}&delay=100&repeat=2&id=1
     dim_matrix: curl -XPATCH http://obegransad.lan/setbrightness\?value\=30
     wake_matrix: curl -XPATCH http://obegransad.lan/setbrightness\?value\=100

And the automation itself

alias: Show Heritage Title
description: ""
trigger:
  - platform: state
    entity_id:
      - media_player.heritage_2
    attribute: media_title
condition: []
action:
  - service: shell_command.show_playing
    data: {}
mode: single

@ph1p any idea if it can be merged?

[ph1p]

Sure! I'll test it tomorrow and merge it afterwards :)

[ph1p]

Many thanks to al of you! :) The PR has been merged. I've updated the code a bit so that each endpoint is now prefixed with /api. I also removed get... because if it's a GET request, it's not necessary.