jjelks: corrected PR - Add to Class(Type) String and DateTime Property functionality (derived from 0.13)

[Jason-Jelks]

Classes can now include String properties by adding the "S7StringAttribute" to the Properties to define maximum string size. The Attribute is required for Class type implementing a String.

Classes can now include DateTime Properties. The internal call structures now support passing the current CPU instantiated so that the proper DateTime structure is implemented based on the PLC. S7-1200 & S71500 : use DateTimeLong (DTL - 12-bytes) all others use: older DT 8-byte format as they don't support the DTL The CPU parameter does not break existing implementations, but does default direct calls made by an external source to S7-300 if not defined.

[scamille]

After a quick preliminary look, the main problem I have here is the following: I don't think it is a good idea to use the CPU type to distinguish between different S7 datetime types to map to .NET datetime. There is S7 DateTime, DateTimeLong and DTL as well. While a S71500 cpu might support DateTimeLong, that does not mean that you can't have any DateTime variables in it.

I would rather propose a custom DateTime attribute, similar to https://github.com/S7NetPlus/s7netplus/blob/develop/S7.Net/Types/S7StringAttribute.cs . When creating the attribute, a user could specify the S7 DateTime type as a enum. Maybe use the good old DateTime S7 type as the default, with DateTimeLong and DTL available as options.

[Jason-Jelks]

Thank you for your feedback. I agree with your thought process, and can certainly make the modification. However I do have a reason for why I chose the path I did; while each of the S7 plc's supports some aspect of [Date_and_Time], not all models support both Simatic Step 7 DataTypes: DT and DTL. The models: S7-200, -300 & -400 do not support DTL format conversion (implicitly or explicitly). The S7-1200 implements DTL format, but does not support DT format conversion according to Siemens documentation. The S7-1500 implements DTL and somewhat recently (2020 I believe) added back in support for DT format conversion.

It would be best not to allow someone to implement something that would knowingly be revoked by the PLC. This is why I went with the approach of passing the assigned CPU to the 'Class' parsing objects. The only time there would be an option for parsing format is the S7-1500, which I will prepare the change you have suggested.

If you have any other suggestions please let me know.

[scamille]

Yeah I know about the mess that is datetime types and the various s7 models. There might definitely be merit to checking if a certain data type is supported by the cpu model targeted. (Either as some kind of warning, or error with opt-out maybe?)

At least in my experience, it is not unlikely to either have some old DB structure ported from 200-400 to 1500 containing normal datetime, or just some plc programmer not liking the new long type. So it should be possible to easily use the datetime type on a 1500 cpu.

@mycroes and @FalcoGoodbody might also have an opinion on this.

[Jason-Jelks]

@scamille I have committed up changes based on your suggestion of adding an attribute for the S7-1500 choices.

At least in my experience, it is not unlikely to either have some old DB structure ported from 200-400 to 1500 containing normal DateTime, or just some plc programmer not liking the new long type. So it should be possible to easily use the DateTime type on a 1500 CPU. I have added the Attribute option, which only benefits the S7-1500. I have left the CPU DI in the methods to avoid any breaking changes. That allows the S7DateTimeAttribute to be optional.

I would love to hear feedback from @mycroes and @FalcoGoodbody if they have additional insights.

[scamille]

Can you please remove UpgradeLog.htm and UpgradeLog2.htm from the PR?

[scamille]

Is there a good reason to prefer DTL instead of LDT for 1500 cpus? I really don't know much about plc programming, but at least from .NET perspective LDT matches Net DateTime a lot better.