Plugin parameter and result chaining

[tisoft]

Allows to add additional in and out options to a Javascript or Go Plugin.

Fixes #5692
Replacement for #5730

[tisoft]

The syntax for the examples has changed. See #7836 (comment) for current syntax.

Examples:

I can get the climate status of my car via MQTT. The value is send as plain string and needs to be converted to a boolean. I can't use jq, since that needs a JSON String. With this change, I can configure the go plugin to do the conversation. The MQTT value is added as the parameter remoteClimateState.

vehicles:
  - type: custom
    title: MG5
    name: ev1
    climater:
      source: go
      vm: shared
      script: |
        remoteClimateState != "off"
      param:
        - name: remoteClimateState
          type: string
          config:
            source: mqtt
            topic: saic/EMAIL/vehicles/VIN/climate/remoteClimateState

I have a go-eCharger, but I can't use the native plug-in, since I can't reach the charger directly from evcc. Both can reach the same MQTT Server though. To enable the charger, I need to convert the true value from evcc to 2. I use a go plug-in to do so and the the result of that plugin to feed it into MQTT. I can do the same for setting the phases1p3p, which requires to convert the 3from evcc to 2before sending to MQTT.

chargers:
  - name: goe1
    type: custom
    enable:
      source: go
      vm: shared
      script: |
        ret := 1
        if enable {
          ret = 2
        }
        ret
      result:
        - name: enable
          type: int
          config:
            source: mqtt
            topic: go-eCharger/XXX/frc/set
    phases1p3p:
      source: go
      vm: shared
      script: |
        ret := 1
        if phases == 3 {
          ret = 2
        }
        ret
      result:
        - name: enable
          type: int
          config:
            source: mqtt
            topic: go-eCharger/XXX/psm/set

[andig]

Really cool PR!

There was a coding style discussion some time ago suggesting to avoid the if err == nil pattern where suitable. Since we're touching large parts here, maybe we should do so where it fits and doesn't make things more complicated.

[tisoft]

The error handling discussion is presumably this one: #6879 Correct?
If so I'll try to use the pattern from there.

[andig]

Yes, thanks for digging that out. Here's one example of a converted setter without named returns:

// NewIntSetterFromConfig creates a IntSetter from config
func NewIntSetterFromConfig(param string, config Config) (func(int64) error, error) {
	factory, err := registry.Get(config.Source)
	if err != nil {
		return nil, err
	}

	provider, err := factory(config.Other)
	if err != nil {
		return nil, err
	}

	prov, ok := provider.(SetIntProvider)
	if !ok {
		return nil, fmt.Errorf("invalid plugin source for type int: %s", config.Source)
	}

	return prov.IntSetter(param), nil
}

/cc @panzerdev

[tisoft]

Changed param -> in and result -> out.

Examples from above would now be:

vehicles:
  - type: custom
    title: MG5
    name: ev1
    climater:
      source: go
      vm: shared
      script: |
        remoteClimateState != "off"
      in:
        - name: remoteClimateState
          type: string
          config:
            source: mqtt
            topic: saic/EMAIL/vehicles/VIN/climate/remoteClimateState

chargers:
  - name: goe1
    type: custom
    enable:
      source: go
      vm: shared
      script: |
        ret := 1
        if enable {
          ret = 2
        }
        ret
      out:
        - name: enable
          type: int
          config:
            source: mqtt
            topic: go-eCharger/XXX/frc/set
    phases1p3p:
      source: go
      vm: shared
      script: |
        ret := 1
        if phases == 3 {
          ret = 2
        }
        ret
      out:
        - name: enable
          type: int
          config:
            source: mqtt
            topic: go-eCharger/XXX/psm/set

Has a better feeling for me, what about you?

[StefanSchoof]

How does this behave, when the source in the param is giving an Error? Is an Error handling in the script possible?

[andig]

Think so. Seems clearer that the top level plugin is not the end of the chain.

[andig]

How does this behave, when the source in the param is giving an Error? Is an Error handling in the script possible?

No- errors are always handled pre/post plugin and break the chain. I'd say that's required if we don't want to have different kinds of plugins (which we don't).

[tisoft]

How does this behave, when the source in the param is giving an Error? Is an Error handling in the script possible?

No custom error handling is possible, the error will bubble up to the top level plugin. In other words, the top level plugin only succeeds if all in plugins succeed.

Unfortunately it's a bit different for out. if you have a sequence of 3 out plugins and the middle one fails, the data of the first will have already been processed and send out. The last one is skipped, the top level fails with the error of the failed plugin.

If you have ideas on how to improve that, I'm all open for it.

[andig]

If you have ideas on how to improve that, I'm all open for it.

Thats just how it is.

No custom error handling is possible, the error will bubble up to the top level plugin. In other words, the top level plugin only succeeds if all in plugins succeed.

That's still not handling. Toplevel logic will only ever be called if all plugins have their data ready? The script never sees the errors. Which is totally fine.

[tisoft]

That's still not handling. Toplevel logic will only ever be called if all plugins have their data ready?

Yes, top-level logic is only called if all in-plugins have delivered data.

[tisoft]

I have extracted the initialization code, so that the in/out plugins are only initialized once. Currently all shared code is in the javascript.go file, I will extract that into it's own file, but wanted to have minimal changes for better review ability first.

Haven't done any other cleanup yet.

[tisoft]

I did some additional cleanup, don't know if that is the right direction though.

Everything is in own commits, so I can go back very easily.

[andig]

I did a bunch of simplifications and style changes, please check if I broke anything.

Last piece i'm confused about is the use of generics. I get the otto vs reflect-ness of the change but do we really need to expose this? Shouldn't it be possible to use the duck-typed Provider interfaces instead? I admit that I didn't try to implement this though.

In other words: we should return the concrete types, not the intermediate internal values. This removes the need for the convertXXX methods of the OutTransformationProvider?

[tisoft]

Thanks for the code improvements, looks really better now. I have compiled it and will test it tomorrow. Will also look into the open review comments.

[tisoft]

Last piece i'm confused about is the use of generics. I get the otto vs reflect-ness of the change but do we really need to expose this? Shouldn't it be possible to use the duck-typed Provider interfaces instead? I admit that I didn't try to implement this though.

In other words: we should return the concrete types, not the intermediate internal values. This removes the need for the convertXXX methods of the OutTransformationProvider?

I haven't found a way to do so, without moving all the type switches into the go/javascript implementations again, which duplicates essentially the same code. But maybe there is a way, to do that cleanly.

Currently we have only two plugins, that support these in/out parameter, but there might be more (thinking about the script plugin, which could set environment variables with in or something like that), and I wanted to have as less code duplication as possible to support such future extensions.

[tisoft]

Tested it today, everything seems to work as expected, so thus would be ready from my point of view.

[andig]

Ich häng noch an der gleichen Stelle. Beispiel go: handleOutTransformation sollte den Wert direkt als Typ any bekommen und nicht ein reflect.Value oder ähnliches. Wir haben den Schnitt an der falschen Stelle.

[tisoft]

Ich bekomme es nicht besser hin. Habe alles mögliche versucht, aber ich bekomme die Typkonvertierung immer nur an der Stelle hin. Die einzige alternative die ich im Moment sehe, wäre das aus dem transformation.go rauszunehmen und in die beiden Implementierungen rein zu-copy-pasten. Aber das finde ich auch nicht wirklich schön. Vielleicht kommt dir ja noch eine Idee...

[andig]

Ich habs versuche, aber ungetestet. Letzter Commit, nur für Go umgesetzt. Magst mal drauf schauen? Wenns nix taugt gerne git revert.

[tisoft]

So, das ist jetzt übers Wochenende bei mir ohne Fehler gelaufen. Habe heute morgen noch ein bisschen refactored. Ist jetzt aus meiner Sicht fertig.

[andig]

Richtig genialer PR, Glückwunsch und Danke!

[andig]

/cc @VolkerK62 damit kann man schön rumspielen und Daten von a nach b schieben, konvertieren etc :)

[andig]

@tisoft hast Du Dich egtl. schon bei Slack angemeldet?

[tisoft]

@tisoft hast Du Dich egtl. schon bei Slack angemeldet?

Jetzt, ja. 😎