[Bug]: Unify the drivers regarding write values acceptance

apache / plc4x

PLC4X The Industrial IoT adapter

https://plc4x.apache.org/

Apache License 2.0

1.24k stars 399 forks source link

[Bug]: Unify the drivers regarding write values acceptance #1718

Open mrwhy-orig opened 1 month ago

mrwhy-orig commented 1 month ago

What happened?

The syntax for writing values differs from driver to driver. Sometimes a primitiv is needed see #1523 otherwise an error is thrown, sometimes a PlcValue Object is needed (ADS) but if not provided no error is thrown, and sometimes primitives and PlcValue objects work (S7).

We should unify this.

Version

v0.13.0

Programming Languages

[X] plc4j
[ ] plc4go
[ ] plc4c
[ ] plc4net

Protocols

[ ] AB-Ethernet
[X] ADS /AMS
[ ] BACnet/IP
[ ] CANopen
[ ] DeltaV
[ ] DF1
[ ] EtherNet/IP
[ ] Firmata
[ ] KNXnet/IP
[ ] Modbus
[X] OPC-UA
[X] S7

splatch commented 1 month ago

This is indeed an issue, because we do not have a clear behavior across drivers. On other hand in many cases we also miss a way to determine what encoding we should use for a tag. To make matter worse, drivers deal with situation where plc4j type (int) can be mapped to several plc types (sint, uint, dint).

From API point of view I believe we could guarantee consistency only when caller provides instance of PlcValue. Wrapper type clearly specifies data type intended by caller, and can be used by driver to assume wire encoding. For other cases we can just try to unify logic through construction of PlcValue in single place. Currently I have impression that our spit PlcValues.of might miss some cases causing to failures.

chrisdutz commented 1 month ago

The main issue I think was that some addresses don't contain the type. Initially we only had the option to add the simple values. But this caused problems, because the driver had no way to see the real type it should send to the PLC.

We were seeing that for example people passed in an "integer", which would be converted to an DINT and when sending a DINT to a field that's an USINT, this causes issues. Also when writing a UDT or an array, we can't pass in a primitives, but we need to pass in PlcList or PlcStruct objects.

In general we would have 2 options:

Extend every address string with the type at the end
Require the user to pass in PlcValues of the expected type

I'm personally leaning more towards the fully qualified address option ... however, as mentioned this fails for structs (For arrays we could find a way around it).