Skip to content

fix(ot-sim): backwards compatibility w/ default DNP3 "scan-rate" option#73

Draft
activeshadow wants to merge 1 commit into
sandialabs:mainfrom
activeshadow:issues/72
Draft

fix(ot-sim): backwards compatibility w/ default DNP3 "scan-rate" option#73
activeshadow wants to merge 1 commit into
sandialabs:mainfrom
activeshadow:issues/72

Conversation

@activeshadow

Copy link
Copy Markdown
Collaborator

Backwards Compatibility for DNP3 Master scan-rate Option

Description

Commit 55d8461 introduced the option to specify a scan rate for DNP3 masters, but did not correctly support the option being missing, breaking backwards compatibility.

Related Issue

#72

Type of Change

Please select the type of change your pull request introduces:

  • Bugfix
  • New feature
  • Documentation update
  • Other (please describe):

Checklist

  • This PR conforms to the process detailed in the Contributing Guide.
  • I have included no proprietary/sensitive information in my code.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have tested my code.

Additional Notes

@tijcolem please do me a favor and test to see if this addresses your issue #72.

Commit 55d8461 introduced the option to specify a scan rate for DNP3
masters, but did not correctly support the option being missing,
breaking backwards compatibility.

fixes sandialabs#72
@activeshadow activeshadow self-assigned this Oct 20, 2025
@GhostofGoes

Copy link
Copy Markdown
Contributor

Awaiting testing from @tjicolem before merging

@tijcolem

Copy link
Copy Markdown

Thanks for putting in the fix. I tested these changes (python 3.10) and hit an issue. I changed the order of the method params and that fixed it.

Using python3.10

Nov 12 10:51:14.102 WRN app returned stderr type=PHENIX-APP stderr="Traceback (most recent call last):  

File \"/usr/local/bin/phenix-app-ot-sim\", line 8, in <module>    sys.exit(main())             ^^^^^^  

File \"/usr/local/lib/python3.12/dist-packages/phenix_apps/apps/otsim/otsim.py\", line 368, in main    OTSim()  

File \"/usr/local/lib/python3.12/dist-packages/phenix_apps/apps/otsim/otsim.py\", line 23, in __init__    self.execute_stage()  

File \"/usr/local/lib/python3.12/dist-packages/phenix_apps/apps/__init__.py\", line 75, in execute_stage    stages_dict[self.stage]()  File \"/usr/local/lib/python3.12/dist-packages/phenix_apps/apps/otsim/otsim.py\", line 297, in pre_start    device.configure(config, ot_devices)  

File \"/usr/local/lib/python3.12/dist-packages/phenix_apps/apps/otsim/device.py\", line 77, in configure    client.init_master_xml(self.configs['scan-rate'])  

File \"/usr/local/lib/python3.12/dist-packages/phenix_apps/apps/otsim/protocols/dnp3.py\", line 55, in init_master_xml    self.master = ET.SubElement(self.root, 'master', {'name': name})                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File \"src/lxml/etree.pyx\", line 3163, in lxml.etree.SubElement  

File \"src/lxml/apihelpers.pxi\", line 199, in lxml.etree._makeSubElement  File \"src/lxml/apihelpers.pxi\", line 194, in lxml.etree._makeSubElement  

File \"src/lxml/apihelpers.pxi\", line 324, in lxml.etree._initNodeAttributes  File \"src/lxml/apihelpers.pxi\", line 335, in lxml.etree._addAttributeToNode  

File \"src/lxml/apihelpers.pxi\", line 1539, in lxml.etree._utf8TypeError: Argument must be bytes or unicode, got 'int'" app=ot-sim action=pre-start exp="&{{5g-badger 2025-11-12T10:51:13-07:00 2025-11-12T10:51:13-07:00 map[phenix.workflow/branch:5g-badger phenix.workflow/tags:method=workflow,dir=/scratch/phenix/topologies/5g-badger,branch=5g-badger,workflow_date=20251112105112MST scenario:5g-badger topology:5g-badger]} 0xc000272000 0xc0001799c0 [{crn28 96 0 [0.21 0.27 0.36] 20197 515482 0 0 0 rx: 0.0 / tx: 0.0 {21 21} 0 0 1.98869721e+06 true true}]}"

Fix.
Change to def init_master_xml(self, scan_rate=5, name='dnp3-master'):

@GhostofGoes

Copy link
Copy Markdown
Contributor

@tijcolem Would you mind opening a new PR with the fix you tested?

@activeshadow

activeshadow commented Dec 10, 2025 via email

Copy link
Copy Markdown
Collaborator Author

@cmulk

cmulk commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

@activeshadow is this still pending a few changes?

@activeshadow

Copy link
Copy Markdown
Collaborator Author

@cmulk yeah I still need to make these changes. I'll try to get to them soon. Thanks for the poke.

@GhostofGoes

Copy link
Copy Markdown
Contributor

Ping @activeshadow, if you're still working on this can you mark as Draft? Gracias.

@activeshadow

Copy link
Copy Markdown
Collaborator Author

Ping @activeshadow, if you're still working on this can you mark as Draft? Gracias.

Will do.

@activeshadow activeshadow marked this pull request as draft June 9, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants