fix: modbusproxy unmarshaling

[Maschga]

Fix #26610

\cc @andig

[Maschga]

@andig How do you think about the fix in 1acb43d?
Doesn't solve the test, but fixes the UI problem.

[andig]

Thats the wrong place. It should be in the unmarshaler.

[Maschga]

That's quite difficult, because unmarshaling is done by calling viper.UnmarshalExact(&conf) which is a library method.
In adittion, it's default behivour to convert true to "1" for a string parameter.

[andig]

I guess the whole problem is this:

type ModbusProxy struct {
	Port            int    `json:"port"`
	ReadOnly        string `yaml:",omitempty" json:"readonly,omitempty"`
	modbus.Settings `mapstructure:",squash" yaml:",inline,omitempty" json:"settings,omitempty"`
}

ReadOnly should be this:

package modbus

// go:generate go tool enumer -type ReadOnlyMode -trimprefix ReadOnly -transform=lower

type `ReadOnlyMode` int

const (
	ReadOnlyFalse ReadOnlyMode = iota
	ReadOnlyDeny               // return modbus error
	ReadOnlyTrue               // silently ignore writes
)

If we add a TextUnmarshaler to ReadOnlyMode this should work.

[andig]

Made a quick test, that seems to work. Now we need to:

• test only the proxy config, not general loadpoint config
• test json
• test marshaling and unmarshaling
• move the test to a better location
• maybe move the type to a better location
• maybe move the proxy config to a better location

[Maschga]

For me, this works for readonly: true but not for readonly: false or readonly: deny.

[andig]

The problem ist that boolean (not string) yaml true gets unmarshaled into bool true which is converted to int(1) which is ReadOnlyDeny==int(1). I've simply changed the order now that make ReadOnlyTrue==int(1). The other problem with using the ReadOnlyType instead of string is that viper doesn't use the additional unmarshal hooks DecodeOther uses. While that can be enforced it seemed much simpler to do it this way, though we can't unit-test viper it now.

[andig]

Hope this works

[andig]

Still not working?

[Maschga]

Haven't had time yet. Just tested it:

readonly: true => "1"
readonly: false => "0"
readonly: deny => "deny"

Is this mix of numbers and strings intentional? If yes, I'll fix the UI.

[andig]

No :(