Push: add custom messenger (BC)

[andig]

Fix #17148 by adding custom messenger. Custom messenger uses encoding to combine title and msg into a single value to send to plugin. Valid encodings are json, csv and tsv. If encoding is omitted only the message is transferred.

BC: removes script messenger. This is replaced by:

- type: custom
  source: script
  encoding: tsv
  cmdline: foo -bar

Replace #17278

[GrimmiMeloni]

Don't we still need the cmdline execution parts from the former script implementation?

[andig]

Bleibt die Frage, ob wir das mergen wollen?

[GrimmiMeloni]

Regarding my previous comment

Don't we still need the cmdline execution parts from the former script implementation?

I now connected the dots, of the registry looking up the provider (factory) via the source field in the config. Not that obvious, took me a moment to grasp.

Bleibt die Frage, ob wir das mergen wollen?

Yes, I think so.

[rhuss]

Nice reuse of the existing plugin interface :) Does this mean that you can reuse any of the plugins as service endpoint for events ? (mqtt, modbus, http, script, ... all plugins that allow write access).

[rhuss]

This is too simplistic, especially when title and message can contain commas (to tabs for tsv). Do we really need csv and tsv ? I can't see any good use case that can not be solved with a stronger typed representation (like json).

[andig]

Does it hurt? Since users do control the message format this can be handled by crafting the right kind of message. Happy to improve this by doing a better encoding.

[andig]

Nvm. We can just use the cvswriter for this as well.

[rhuss]

Does it hurt? Since users do control the message format this can be handled by crafting the right kind of message. Happy to improve this by doing a better encoding.

Well, I've been in the open-source software business for too long, I guess. I've learned over the long run to respect YAGNI and better leave out things than anticipate anything the people might use at some point in time. It's easy to add these features later anyway if needed, but if nobody uses them, you will carry that baggage with you forever. Ideally, you add some tests for it (especially for corner cases and strange escape rules), and you need some documentation. I became a fanboy of less is more most of the time.

Or let me ask the other way around: Which use case do you think needs CSV, TSV, or title—only for sending out to an endpoint? Couldn't you just do it all yourself when defining the msg, even with the proper templating support?

In the end, it's your call anyway, as I'm like many other contributors: Here for some time to help, but likely not forever. Just sharing some stuff.

[andig]

Or let me ask the other way around: Which use case do you think needs CSV, TSV, or title—only for sending out to an endpoint?

Anybody using the script messenger right now.

[rhuss]

I don't think so.

The script provider does not implement SetStringProvider (so it wouldn't work anyway as is). I would keep it simplistic and ignore the title.

Empty titles will lead to strange CSV and TSV representations (like a leading , or leading tab), too. Still, we should allow for empty titles for this kind of integration, as this would be the most natural way to define no title and a JSON msg when using a custom provider (you can always add a title: in that msg that the user defines if you like)

[rhuss]

Using tsv with

cmdline: script ${send}

Getting title and message as two separate arguments will not work when $msg (or $title) contains spaces.

So I think that most of the time, one would use cmdline: 'script "${send}"' to get the whole input as a single argument to the script. I would always leave out the title for custom providers.

[rhuss]

this type does not "fit" into the list: json, csv, tsv are formats (so kind of an encoding), "title" and "message" (the default) are more filters.

Wdyt to restrict encoding to "json" and "string" (or "raw") and have some second options as to whether to include the title or not ? (usually if you only use a single service endpoint and you don't need the title, you just don't define it in events: ...

[andig]

this type does not "fit" into the list: json, csv, tsv are formats (so kind of an encoding), "title" and "message" (the default) are more filters.

It doesn't, but it allows returning the title instead of the message for simple command line scripts. It doesn't hurt anyway.

[rhuss]

I still don't buy the argument it doesn't hurt anyway, but your mileage may vary :-)

[rhuss]

Another question: Shouldn't the config look like

- type: custom
  send: 
    source: script
    cmdline: foo -bar "${send}"
  encoding: tsv

[rhuss]

The following configuration worked for me with this PR:

# MQTT Broker Configuration
mqtt:
  broker: broker.mqtt:1883
  user: evcc
  password: ...
messaging:
  events:
    connect:
      msg: '{"car": "${vehicleTitle}", "mode": "${mode}", "type": "connect"}'
    disconnect:
      msg: '{"car": "${vehicleTitle}", "mode": "${mode}", "type": "disconnect"}'
  services:
    - type: custom
      send: 
        source: mqtt
        topic: "evcc/events"

[GrimmiMeloni]

FYI, the following configuration worked for me with this PR:

Thanks for trying it out. 👍🏻🙏🏻

[rhuss]

Thanks @andig for merging 🙏 . Would it makes sense, if I submit a PR for evcc-docs to document this feature ? (while I'm still kind of in the weeds).

[andig]

@rhuss that would be great! Tbo, docs are my personally weakest spot...

[RenatusRo]

mh, need a doc.
hab' so ziemlich jede permutation von send, source und cmdline probiert, aber kommt stets

failed configuring push service custom: cannot create messenger type 'custom': decoding failed due to the following error(s):
'' has invalid keys: cmdline

    - type: custom
      send:
        source: script             
        cmdline: /home/rro/evcc-config/evcc_message
      encoding: csv

[rhuss]

@RenatusRo hast Du von diesem Branch gebaut oder nach dem es gemerged wurde, von master ? Es koennte sein, das auf dem Branch noch der PR #17314 fehlt. Am bestem also master updaten und nochmal bauen.

script habe ich allerdings nicht getestet, mit mqtt hats bei mir aber funktioniert.

[RenatusRo]

war vom master.

[RenatusRo]

vermute, cmd is the new cmdline.

    - type: custom
      send:
        source: script             
        cmd: /home/rro/evcc-config/evcc_message

[rhuss]

@RenatusRo ja, richtig. Es wird jetzt der gleiche Konfigurationssyntax wie bei dem Script plugin genutzt (beschrieben in https://docs.evcc.io/docs/reference/plugins#shell-script-lesenschreiben), d.h.

cmd: myscript.sh "{{.send}}"
scale: 1
timeout: 30s
cache: 30s

(und zusaetzlich eins von Regex, Jq, Unpack, Decode und Default um beim Lesen den eigentlich Wert zu extrahieren).

Ich wuerde kein encoding setzen, dann wird der in msg: definierte Wert komplett als ein Argument an myscript.sh eingesetzt. Ansonsten musst du selbst im Script wieder titel & message auseinander trennen (und selbst wenn Du den titel leer laesst, bekommst Du ueberfluessige Trennzeichen aktuell, mit einer leeren ersten Spalte. Warum ich kein Fan von csv und tsv encoding bin hab ich ja schon dargelegt ;-)

In deinem Beispiel koenntest Du es mit dem folgender Konfiguration probieren:

messaging:
  events:
    connect:
      msg: '{"car": "${vehicleName}", "mode": "${mode}", "type": "connect"}'
    disconnect:
      msg: '{"car": "${vehicleName}", "mode": "${mode}", "type": "disconnect"}'
  services:
    - type: custom
      send: 
        source: script
        cmd: /home/rro/evcc-config/evcc_message "{{.send}}"

und in dem Script kannst Du dann mit jq die einzelnen werte herausfischen (falls Du das brauchst).

[rhuss]

@andig nur eine Frage aus Neugierde: Warum verwenden der ScriptProvider html/template und nicht einfach text/template ? Ich befuerchte das HTML-escaping kann in dem Kontext schon zu Problemen fuehren (bzw. ueberaschende Effekt haben), falls das escaping getriggert wird (z.b. wenn bei dem messaging beispiel XML als input definiert wird, msg: '<car>${vehicleName}</car>'

Auch fehlt irgendwie die komplette Dokumentation, wie man parameter mit dem {{.var}} syntax einfuegt, das einzige was ich gefunden habe war ${enable:%b} was wohl laut code ein aelterer Syntax ist, der zusaetzlich zum Gotemplate syntax angewendet wird (sollte man das nicht besser separieren und nicht chainen?).

[rhuss]

Ich weiss nicht, wie der Prozess ist, aber vielleicht sollte man den urspruenglichen script service nicht gleich entfernen, da der Ersatz ja bzgl. der Konfiguration kein 1:1 replacement ist. Vielleicht erstmal deprecaten und in einer spaeteren Version loeschen ?

[RenatusRo]

scheinbar ist ausser mir noch niermand drueber gestolptert - no issues yet :-)

[rhuss]

Ja, sind vielleicht nur 'corner cases', und vielleicht ist ja der happy-path breit genug :-)

[andig]

Warum verwenden der ScriptProvider html/template und nicht einfach text/template

Vmtl. ein Versehen, gerne ändern!

[andig]

Ich weiss nicht, wie der Prozess ist, aber vielleicht sollte man den urspruenglichen script service nicht gleich entfernen,

Den Messenger? Den hat der PR doch schon entfernt?

[rhuss]

ok, ich schau mal wegen html/template in util.ReplaceFormatted ersetzen durch text/template.

Den Messenger? Den hat der PR doch schon entfernt?

Ja, genau. Ich dachte, vielleicht sollte man ihn vor der naechsten Release wieder zurueckholen, damit die Nutzer Zeit haben zum upgraden.

---

btw, ich in https://github.com/rhuss/evcc-charging-planner mal einen prototypen für einen Ladewochenplaner gebastelt, der den mqtt custom messenger benutzt.

[rhuss]

@andig text/template PR --> #17358

[rhuss]

@rhuss that would be great! Tbo, docs are my personally weakest spot...

@andig here we go --> evcc-io/docs#675