tnunnink / L5Sharp

A library for intuitively interacting with Rockwell's L5X import/export files.
MIT License
55 stars 6 forks source link

Inconsistent use of bool and BOOL data type #27

Closed TunedCanna closed 6 months ago

TunedCanna commented 6 months ago

Hi Tnunnink,

First of all... Awesome project, lots of great stuff here!

I'm having some trouble loading some L5X files that have been generated by Rockwell's Application Code Manager. I'm sure you're aware that ACM creates L5X files which are a bit inconsistent in the use of "0"/"1" or "false"/"true". Logix Designer is similarly inconsistent when importing these L5X files.

Here's an example of a controller element in an L5X file:

<Controller ... ShareUnusedTimeSlice="0" ... ReportMinorOverflow="false">

This will happily import into Logix Designer but will throw an error when loaded as a L5X object as it expects the ShareUnusedTimeSlice property to be a bool.

When changed to this:

<Controller ... ShareUnusedTimeSlice="true" ... ReportMinorOverflow="false">

The opposite is true - failing to import into Logix Designer but happily loaded as an L5X object.

I've had this problem with Controller's ShareUnusedTimeSlice property and Program's Disabled property so far.

I see the BOOL.cs class that is being used as a solution to this in a variety of places. Would be happy to make a PR where I fix some problematic properties but thought I'd first pop by to see if you had any insight to offer as to why it's not more widely used and/or if there's a better approach.

Cheers!

tnunnink commented 6 months ago

Thanks for sharing this. Just to make sure I understand, you are saying ACM is what is failing to load the Controller element when ShareUnusedTimeSlice is 0/1 instead of false/true, not Studio?

In any case, I can add a fix for this. I know that Rockwell is inconsistent with its Boolean values. I think I would prefer to add something to LogixElement or LogixParser instead of replacing these with a BOOL type, since BOOL is meant for Tag data elements.

tnunnink commented 6 months ago

Oh sorry, you are saying the L5X object cannot parse 0/1 as a bool and is throwing an exception, correct?

TunedCanna commented 6 months ago

Yes, that's right. I'm attempting to load an L5X file into an L5X object and an exception is being thrown where a "0" or "1" is found where a bool was expected.

So far I've noticed it in two places.

The Disabled property of Program, and the ShareUnusedTimeSlice property of Controller.

If I modify the L5X file to change these to be "true" or "false" instead then they will fail to load in Studio.

Have found it quite difficult to pin down how ACM decides, when generating an L5X file, which properties are "0"/"1" or "true"/"false". It almost seems like [BOOL](public sealed class BOOL : AtomicData, IComparable, IConvertible, ILogixParsable) should be used in most places.

tnunnink commented 6 months ago

I see. Thanks for clarifying. I'm adding a couple things to fix this.

For reading Booleans, I am adding a custom parse function to LogixParser to handle properties/attributes that are being parsed as bool, in a similar way to BOOL. This will fix issues with L5X ability to read 0/1 and covert that to true/false, and we can leave properties as bool in this case.

For writing Booleans, I'm adding a new set method the LogixElement called SetBit which will write 0/1 using the provided Boolean value. This will have to replace the SetValue method for each property/attribute that requires the 0/1 instead of true/false. Using BOOL here doesn't necessarily fix the problem, since we would still need to decide which value to write. BOOL just handles parsing.

In regards to the Disable property for Program, Rockwell's documentation says it serializes as true/false. image

When I export a Program, I see true/false as well.

<Programs Use="Context">
<Program Use="Target" Name="Empty" TestEdits="false" Disabled="false" UseAsFolder="false">
<Tags/>
<Routines/>
</Program>

But I tested changing this to 1 and it imports just fine and indeed inhibits the program. Out of curiosity, which version of Studio are you using? I suppose I can change Disable to be 1/0 but it's strange that it's not working as true/false for you.

tnunnink commented 6 months ago

Version 2.3.1 adds the fixes described above and implements the SetBitfor ShareUnusedTimeSlice.

I added a test to verify but let me know if you still have issues or not.

TunedCanna commented 6 months ago

Big appreciation for being so quick to respond and implement. Thank you!

I'm not all too familiar with Rockwell's application suite so crossing my fingers I've been getting their names right.

Studio 5000 Application Code Manager says Version 4.03.00. This is what is generating my L5X file. Studio 5000 Logix Designer says 32.03.00.

Will give 2.3.1 a whirl and report back with how it goes :)

tnunnink commented 6 months ago

No problem! Thanks for sharing this issue. I don't have time to test exporting/importing every single thing, so you finding this helps this library become better.

TunedCanna commented 6 months ago

Beautifully done! That's every L5X file that I have on hand being loaded with seeming no problems at all.

Again, thank you a lot for your efforts here - it's greatly appreciated!