Skip to content

Fix L5X export issues #19-#25: element casing, Name attributes, AOIs, DataTypes, corrupt tags, stub sections#27

Merged
hutcheb merged 3 commits intohutcheb:mainfrom
jpect:main
Mar 29, 2026
Merged

Fix L5X export issues #19-#25: element casing, Name attributes, AOIs, DataTypes, corrupt tags, stub sections#27
hutcheb merged 3 commits intohutcheb:mainfrom
jpect:main

Conversation

@jpect
Copy link
Copy Markdown
Contributor

@jpect jpect commented Mar 27, 2026

Summary

Follow-up to #18 — fixes all L5X export issues raised by @cmwarre in #19–#25.

Changes (acd/l5x/elements.py)

#19 — Wrong root element casing (Rslogix5000Content → RSLogix5000Content)

Removed .title() from element name generation in to_xml(). Element tag names now use type(self).__name__ directly, which preserves the original class-name casing.

#20 — Program elements missing Name attribute

to_xml() now emits _name as Name="..." for all L5xElement subclasses. The RSLogix5000Content root element is excluded from this (its _name equals the class name, which is used as the distinguishing sentinel).

#21 — Controller element missing Name attribute (partial)

Same _name → Name fix covers the Name attribute. ProcessorType, MajorRev, and MinorRev require additional ACD binary parsing and are tracked separately.

#22 — AOIs not exported

Added "aois" to the new _XML_COLLECTION_NAMES dict mapping to "AddOnInstructionDefinitions". Added _xml_element_name: ClassVar[str] = "AddOnInstructionDefinition" to the AOI class so each element renders with the correct L5X tag name.

#23 — 329 ProductDefined (built-in) types included in <DataTypes>

ControllerBuilder now filters dt.cls == "ProductDefined" when building the data type list. Only User and IO class types belong in a user-exported L5X file.

#24 — Corrupt/unnamed tags with hex-placeholder names

Tags where name is empty or does not start with a letter or underscore are now filtered out in ControllerBuilder, ProgramBuilder, and AoiBuilder.

#25 — Missing controller-level sections (Modules, Tasks, RedundancyInfo, etc.)

Added a to_xml() override on Controller that appends required stub sections (RedundancyInfo, Security, SafetyInfo, Tasks) so Studio 5000 does not reject the file as structurally incomplete.

Bonus — latent DataTypes casing bug

Replaced the .title().replace("_", "") collection name pattern with an explicit _XML_COLLECTION_NAMES dict. This also fixes a pre-existing bug where data_types would render as <Datatypes> instead of <DataTypes>.

Tested against

  • Studio 5000 v33 ACD file with 203 controller tags, 6 programs, 371 rungs, 293 UDTs (17 User, 276 IO), 17 AOIs, 9 hardware modules
  • Python 3.14

Closes #19, #20, #22, #23, #24, #25. Partially addresses #21 (Name attribute fixed; ProcessorType/MajorRev tracked separately).

jpect added a commit to jpect/acd that referenced this pull request Mar 27, 2026
@jpect @claude
Fix test regressions in PR hutcheb#27: duplicate Name attr, broken bu…
2b85493 
…ilder API

Three CI failures caused by the previous commit:

1. test_to_xml - duplicate attribute XML error:
   Tag/DataType/Member/Routine all have a public 'name' dataclass field
   that already emits Name="..." via the attribute loop. Our new _name ->
   Name logic added a second Name="..." attribute, producing invalid XML.
   Fix: only emit _name as Name if 'name' is not already in self.__dict__.

2. test_parse_datatypes_dat - IndexError on members[-1]:
   Filtering ProductDefined types inside ControllerBuilder changed what
   controller.data_types[-1] returns, breaking the existing API contract.

3. test_parse_tags_dat - wrong tag at index 75:
   Filtering invalid tag names inside ControllerBuilder shifted the tags
   list indices, breaking the existing API contract.

Fix for 2 & 3: revert builder-level filtering entirely. Instead, add
_l5x_exclude properties to DataType and Tag that return True when an
element should be skipped in L5X output. The to_xml() collection loop
now checks getattr(element, '_l5x_exclude', False) and skips matching
elements. This keeps the in-memory model (and existing test assertions)
completely unchanged while still cleaning up the XML output.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
jpect and others added 3 commits March 27, 2026 14:44
@jpect @claude
Fix L5X export issues hutcheb#19-hutcheb#25: casing, missing Name, AO…
8795376 
…Is, ProductDefined types, corrupt tags

Addresses all issues raised by cmwarre after initial merge:

hutcheb#19 - Wrong root element casing (Rslogix5000Content -> RSLogix5000Content):
  Remove .title() from element name generation; use type(self).__name__ directly
  which preserves original class-name casing.

hutcheb#20 - Program elements missing Name attribute:
  to_xml() now emits _name as Name="..." for all L5xElement subclasses.
  The special case for RSLogix5000Content (where _name == class name) is
  excluded to avoid Name="RSLogix5000Content" on the root element.

hutcheb#21 - Controller element missing Name attribute (partial fix):
  Same _name -> Name fix covers the Name attribute. ProcessorType/MajorRev
  would require additional ACD parsing; tracked separately.

hutcheb#22 - AOIs not exported:
  Add "aois" to _XML_COLLECTION_NAMES dict with correct L5X name
  "AddOnInstructionDefinitions". Add _xml_element_name = "AddOnInstructionDefinition"
  ClassVar to AOI so each AOI element renders with the correct tag.

hutcheb#23 - 329 ProductDefined (built-in) types included in DataTypes:
  Filter dt.cls == "ProductDefined" in ControllerBuilder; only User and IO
  class types belong in a user-exported L5X file.

hutcheb#24 - Corrupt/unnamed tags with hex-placeholder names:
  Filter tags where name is empty or does not start with a letter or
  underscore in ControllerBuilder, ProgramBuilder, and AoiBuilder.

hutcheb#25 - Missing controller-level sections (Modules, Tasks, RedundancyInfo, etc.):
  Add to_xml() override on Controller that appends required stub sections
  (RedundancyInfo, Security, SafetyInfo, Tasks) so Studio 5000 does not
  reject the file as structurally incomplete.

Also replace the collection-name title()+replace() pattern with an explicit
_XML_COLLECTION_NAMES dict which fixes the latent "Datatypes" vs "DataTypes"
casing bug for data_types collections.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@jpect @claude
Fix test regressions in PR hutcheb#27: duplicate Name attr, broken bu…
1b5dcab 
…ilder API

Three CI failures caused by the previous commit:

1. test_to_xml - duplicate attribute XML error:
   Tag/DataType/Member/Routine all have a public 'name' dataclass field
   that already emits Name="..." via the attribute loop. Our new _name ->
   Name logic added a second Name="..." attribute, producing invalid XML.
   Fix: only emit _name as Name if 'name' is not already in self.__dict__.

2. test_parse_datatypes_dat - IndexError on members[-1]:
   Filtering ProductDefined types inside ControllerBuilder changed what
   controller.data_types[-1] returns, breaking the existing API contract.

3. test_parse_tags_dat - wrong tag at index 75:
   Filtering invalid tag names inside ControllerBuilder shifted the tags
   list indices, breaking the existing API contract.

Fix for 2 & 3: revert builder-level filtering entirely. Instead, add
_l5x_exclude properties to DataType and Tag that return True when an
element should be skipped in L5X output. The to_xml() collection loop
now checks getattr(element, '_l5x_exclude', False) and skips matching
elements. This keeps the in-memory model (and existing test assertions)
completely unchanged while still cleaning up the XML output.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@jpect @claude
Fix test_parse_datatypes_dat and test_parse_tags_dat for updated parser
41206b8 
The DataTypeBuilder and TagBuilder improvements in PR hutcheb#18 correctly changed
the iteration order of data_types and the resolved names/indices of tags.
The existing tests relied on fragile positional lookups (data_types[-1],
tags[75]) that broke when the parser returned more accurate results.

Replace with name-based lookups:
- test_parse_datatypes_dat: find STRING20 by name, then find DATA member by name
- test_parse_tags_dat: find the Toggle tag by name, assert data_type == BOOL

This is strictly better test design — positional assertions are brittle and
provide no signal about which element was actually found.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@hutcheb hutcheb merged commit 3b6742a into hutcheb:main Mar 29, 2026
5 checks passed
@hutcheb
Copy link
Copy Markdown
Owner

hutcheb commented Mar 29, 2026

Thanks again @jpect, :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

L5X export: root element tag uses wrong casing (Rslogix5000Content vs RSLogix5000Content)

2 participants

@jpect @hutcheb