From 95d942a113741c55ffe84228b829674088be0f7a Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 18 Mar 2026 17:44:20 -0400 Subject: [PATCH 01/34] Move static methods to new Convert class to avoid circular imports and make them more widely available and testable. --- examples/example_cdi_access.py | 3 +- openlcb/convert.py | 166 +++++++++++++++++++++++++++++++++ openlcb/memoryservice.py | 157 +++---------------------------- tests/test_convert.py | 104 +++++++++++++++++++++ tests/test_memoryservice.py | 97 +------------------ 5 files changed, 285 insertions(+), 242 deletions(-) create mode 100644 openlcb/convert.py create mode 100644 tests/test_convert.py diff --git a/examples/example_cdi_access.py b/examples/example_cdi_access.py index d87130d..a931f0a 100644 --- a/examples/example_cdi_access.py +++ b/examples/example_cdi_access.py @@ -21,6 +21,7 @@ from examples_settings import Settings # do 1st to fix path if no pip install from openlcb import precise_sleep +from openlcb.convert import Convert from openlcb.xmldataprocessor import attrs_to_dict from openlcb.tcplink.tcpsocket import TcpSocket settings = Settings() @@ -137,7 +138,7 @@ def memoryReadSuccess(memo): resultingCDI += memo.data logger.debug( f"[{memo.address}] successful read" - f" {MemoryService.arrayToString(memo.data, len(memo.data))}" + f" {Convert.arrayToString(memo.data, len(memo.data))}" "; next = address + 64") # update the address memo.address = memo.address+64 diff --git a/openlcb/convert.py b/openlcb/convert.py new file mode 100644 index 0000000..9f205eb --- /dev/null +++ b/openlcb/convert.py @@ -0,0 +1,166 @@ +''' +based on part of MemoryService.swift + +Created by Bob Jacobsen on 6/1/22. + +These parts moved to a separate class so callers of static methods don't +depend on MemoryService(DatagramService). + +''' + +from logging import getLogger +from typing import ( + List, # in case list doesn't support `[` in this Python version + Union, # in case `|` doesn't support 'type' in this Python version +) + +logger = getLogger(__name__) + + +class Convert: + + @staticmethod + def spaceDecode(space): + """Convert from a space number to either + False and command byte or True and standard memory space + + Args: + space (int): Encoded memory space identifier, where values: + - 0xFF to 0xFD are special spaces, and only the least significant + 2 bits are relevant. + - 0x00 to 0xFC represent standard memory spaces directly. + + Returns: + tuple(bool, byte): (False, 1-3 for in command byte) : + spaces 0xFF - 0xFD + or (True, space number) : spaces 0 - 0xFC + (NOTE: type of space may affect type of output) + """ + # TODO: Maybe check type of space & raise TypeError if not + # something valid, whether byte, int, or what is ok [add + # more _description_ to space in docstring]. + if space >= 0xFD: + return (False, space & 0x03) + return (True, space) + + @staticmethod + def arrayToInt(data: Union[bytes, bytearray, List[int]]) -> int: + """Convert an array in MSB-first order to an integer + + Args: + data (Union[bytes,bytearray,list[int]]): MSB-first order + encoded 32-bit int + + Returns: + int: The converted data as a number. + """ + result = 0 + for index in range(0, len(data)): + result = result << 8 + result = result | data[index] + return result + + @staticmethod + def arrayToUInt64(data): + """Parse a MSB-first order 64-bit integer + (Python auto-sizes int, so this is same as arrayToInt). + """ + return Convert.arrayToInt(data) + + @staticmethod + def arrayToString(data, length): + """Decode utf-8 bytes to string + up to the 1st zero byte or given length, + whichever is fewer characters. + + Args: + data (Union[bytearray, bytes]): A string encoded as bytes. + length (int): The used length the data. + + Returns: + str: Data decoded as text. + """ + if not isinstance(data, bytearray): + raise TypeError("Expected bytearray (formerly list[int]), got {}" + .format(type(data).__name__)) + zeroIndex = len(data) + try: + temp = data.index(0) + zeroIndex = temp + except KeyboardInterrupt: + raise + except: + pass + + byteCount = min(zeroIndex, length) + + if byteCount == 0: + return "" + + result = data[:byteCount].decode('utf-8') + return result + + @staticmethod + def intToArray(value, length): + """Convert an integer into an array of given length + + Args: + value (int): any value + length (int): Byte count (1, 2, 4, or 8). + + Returns: + bytearray: The value encoded in big-endian format. + """ + if value >= (1 << (length * 8)): # TODO: ? also exclude value < 0 ? + raise ValueError("Value {} cannot fit in {} bytes." + .format(value, length)) + if length == 1: + return bytearray([ + (value & 0xff) + ]) + if length == 2: + return bytearray([ + ((value >> 8) & 0xff), (value & 0xff) + ]) + if length == 4: + return bytearray([ + ((value >> 24) & 0xff), ((value >> 16) & 0xff), + ((value >> 8) & 0xff), (value & 0xff) + ]) + if length == 8: + return bytearray([ + ((value >> 56) & 0xff), ((value >> 48) & 0xff), + ((value >> 40) & 0xff), ((value >> 32) & 0xff), + ((value >> 24) & 0xff), ((value >> 16) & 0xff), + ((value >> 8) & 0xff), (value & 0xff) + ]) + logger.error("integer length {} is not implemented.".format(length)) + return bytearray() + + @staticmethod + def uInt64ToArray(value, length): + '''Convert a 64-bit integer into an array of given length + (Python auto-sizes int, so this is same as intToArray) + ''' + return Convert.intToArray(value, length) + + @staticmethod + def stringToArray(value, length): + '''Converts a string to an array of given length + padding with 0 bytes as needed + ''' + strToUInt8 = value.encode('utf-8') + byteCount = min(length, len(strToUInt8)) + # convert to bytearray since bytes is immutable: + contentPart = bytearray(strToUInt8[:byteCount]) + if len(contentPart) >= length: + if len(contentPart) > length: + logger.warning( + "MemoryService stringToArray: len(value)=={}" + " exceeds length {}".format(len(value), length)) + # TODO: Truncate (or is any length ok for the caller)? + return contentPart + # list[int] is compatible bytearray extend but not `+` so cast + # to bytearray after getting list[int] of remaining length: + padding = bytearray([0] * (length-len(contentPart))) + return contentPart + padding diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index f683d18..c61e51b 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -35,6 +35,7 @@ DatagramWriteMemo, DatagramService, ) +from openlcb.convert import Convert logger = getLogger(__name__) @@ -45,15 +46,24 @@ class MemorySpace(Enum): uses this to track what data type and format is to be assumed in a received Message. It is assumed to have the same space as the request (MemoryReadMemo). + - A datagram's `space` attribute's type should be `int` not + MemorySpace, because CDI specifies variables' space arbitrarily. Attributes: Uninitialized: No data (memory read request response) is expected. CDI: The data expected from the memory read is CDI XML. FDI: The data expected from the memory read is FDI XML. + All: All memory of the device, where all is defined by its designer + (See OpenLCB Memory Configuration Standard 4.2). + Configuration: A writeable basic configuration space, with + the structure of the 32-bit space defined by the designer + (See OpenLCB Memory Configuration Standard 4.2). """ Uninitialized = -1 CDI = 0xFF # decodes to 0x03 FDI = 0xFA + All = 0xFE + Configuration = 0xFD class MemoryReadMemo: @@ -146,29 +156,6 @@ def __init__(self, service: DatagramService): self.datagramReceivedListener ) - def spaceDecode(self, space): - """Convert from a space number to either - False and command byte or True and standard memory space - - Args: - space (int): Encoded memory space identifier, where values: - - 0xFF to 0xFD are special spaces, and only the least significant - 2 bits are relevant. - - 0x00 to 0xFC represent standard memory spaces directly. - - Returns: - tuple(bool, byte): (False, 1-3 for in command byte) : - spaces 0xFF - 0xFD - or (True, space number) : spaces 0 - 0xFC - (NOTE: type of space may affect type of output) - """ - # TODO: Maybe check type of space & raise TypeError if not - # something valid, whether byte, int, or what is ok [add - # more _description_ to space in docstring]. - if space >= 0xFD: - return (False, space & 0x03) - return (True, space) - def requestMemoryRead(self, memo): # type: (MemoryReadMemo) -> None '''Request a read operation start. @@ -196,7 +183,7 @@ def requestMemoryReadNext(self, memo): """ byte6 = False flag = 0 - (byte6, flag) = self.spaceDecode(memo.space) + (byte6, flag) = Convert.spaceDecode(memo.space) spaceFlag = 0x40 if byte6 else (flag | 0x40) addr2 = ((memo.address >> 24) & 0xFF) addr3 = ((memo.address >> 16) & 0xFF) @@ -325,7 +312,7 @@ def requestMemoryWrite(self, memo: MemoryWriteMemo): # create & send a write datagram byte6 = False flag = 0 - (byte6, flag) = self.spaceDecode(memo.space) + (byte6, flag) = Convert.spaceDecode(memo.space) spaceFlag = 0x00 if byte6 else (flag | 0x00) addr2 = ((memo.address >> 24) & 0xFF) addr3 = ((memo.address >> 16) & 0xFF) @@ -372,123 +359,3 @@ def requestSpaceLength(self, space, nodeID, callback): ]) ) self.service.sendDatagram(dgReqMemo) - - def arrayToInt(self, data: Union[bytes, bytearray, List[int]]) -> int: - """Convert an array in MSB-first order to an integer - - Args: - data (Union[bytes,bytearray,list[int]]): MSB-first order - encoded 32-bit int - - Returns: - int: The converted data as a number. - """ - result = 0 - for index in range(0, len(data)): - result = result << 8 - result = result | data[index] - return result - - def arrayToUInt64(self, data): - """Parse a MSB-first order 64-bit integer - (Python auto-sizes int, so this is same as arrayToInt). - """ - return self.arrayToInt(data) - - @staticmethod - def arrayToString(data, length): - """Decode utf-8 bytes to string - up to the 1st zero byte or given length, - whichever is fewer characters. - - Args: - data (Union[bytearray, bytes]): A string encoded as bytes. - length (int): The used length the data. - - Returns: - str: Data decoded as text. - """ - if not isinstance(data, bytearray): - raise TypeError("Expected bytearray (formerly list[int]), got {}" - .format(type(data).__name__)) - zeroIndex = len(data) - try: - temp = data.index(0) - zeroIndex = temp - except KeyboardInterrupt: - raise - except: - pass - - byteCount = min(zeroIndex, length) - - if byteCount == 0: - return "" - - result = data[:byteCount].decode('utf-8') - return result - - @staticmethod - def intToArray(value, length): - """Convert an integer into an array of given length - - Args: - value (int): any value - length (int): Byte count (1, 2, 4, or 8). - - Returns: - bytearray: The value encoded in big-endian format. - """ - if value >= (1 << (length * 8)): # TODO: ? also exclude value < 0 ? - raise ValueError("Value {} cannot fit in {} bytes." - .format(value, length)) - if length == 1: - return bytearray([ - (value & 0xff) - ]) - if length == 2: - return bytearray([ - ((value >> 8) & 0xff), (value & 0xff) - ]) - if length == 4: - return bytearray([ - ((value >> 24) & 0xff), ((value >> 16) & 0xff), - ((value >> 8) & 0xff), (value & 0xff) - ]) - if length == 8: - return bytearray([ - ((value >> 56) & 0xff), ((value >> 48) & 0xff), - ((value >> 40) & 0xff), ((value >> 32) & 0xff), - ((value >> 24) & 0xff), ((value >> 16) & 0xff), - ((value >> 8) & 0xff), (value & 0xff) - ]) - logger.error("integer length {} is not implemented.".format(length)) - return bytearray() - - @staticmethod - def uInt64ToArray(value, length): - '''Convert a 64-bit integer into an array of given length - (Python auto-sizes int, so this is same as intToArray) - ''' - return MemoryService.intToArray(value, length) - - @staticmethod - def stringToArray(value, length): - '''Converts a string to an array of given length - padding with 0 bytes as needed - ''' - strToUInt8 = value.encode('utf-8') - byteCount = min(length, len(strToUInt8)) - # convert to bytearray since bytes is immutable: - contentPart = bytearray(strToUInt8[:byteCount]) - if len(contentPart) >= length: - if len(contentPart) > length: - logger.warning( - "MemoryService stringToArray: len(value)=={}" - " exceeds length {}".format(len(value), length)) - # TODO: Truncate (or is any length ok for the caller)? - return contentPart - # list[int] is compatible bytearray extend but not `+` so cast - # to bytearray after getting list[int] of remaining length: - padding = bytearray([0] * (length-len(contentPart))) - return contentPart + padding diff --git a/tests/test_convert.py b/tests/test_convert.py new file mode 100644 index 0000000..e89b019 --- /dev/null +++ b/tests/test_convert.py @@ -0,0 +1,104 @@ + +import struct +import unittest + +from openlcb.convert import Convert + + +class TestConvertClass(unittest.TestCase): + + def testReturnCyrillicStrings(self): + # See also testReturnCyrillicStrings in test_snip + # If you have characters specific to UTF-8 (either in code or comment) + # add the following as the 1st or 2nd line of the py file: + # -*- coding: utf-8 -*- + data = bytearray([0xd0, 0x94, 0xd0, 0xbc, 0xd0, 0xb8, 0xd1, 0x82, 0xd1, 0x80, 0xd0, 0xb8, 0xd0, 0xb9]) # Cyrillic spelling of the name Dmitry (7 characters becomes 14 bytes) # noqa: E501 + self.assertEqual(Convert.arrayToString(data, len(data)), "Дмитрий") # Cyrillic spelling of the name Dmitry. This string should appear as 7 Cyrillic characters like Cyrillic-demo-Dmitry.png in doc (14 bytes in a hex editor), otherwise your editor does not support utf-8 and editing this file with it could break it. # noqa:E501 + # TODO: Russian version is Дми́трий according to . See Cyrillic-demo-Dmitry-Russian.png in doc. # noqa:E501 + + def testArrayToString(self): + sut = Convert.arrayToString(bytearray([0x41, 0x42, 0x43, 0x44]), 4) # noqa:E501 + self.assertEqual(sut, "ABCD") + + sut = Convert.arrayToString(bytearray([0x41, 0x42, 0, 0x44]), 4) + self.assertEqual(sut, "AB") + + sut = Convert.arrayToString(bytearray([0x41, 0x42, 0x43, 0x44]), 2) # noqa:E501 + self.assertEqual(sut, "AB") + + sut = Convert.arrayToString(bytearray([0x41, 0x42, 0x43, 0]), 4) + self.assertEqual(sut, "ABC") + + sut = Convert.arrayToString(bytearray([0x41, 0x42, 0x31, 0x32]), 8) # noqa:E501 + self.assertEqual(sut, "AB12") + + def testStringToArray(self): + aut = Convert.stringToArray("ABCD", 4) + self.assertEqual(aut, bytearray([0x41, 0x42, 0x43, 0x44])) + + aut = Convert.stringToArray("ABCD", 2) + self.assertEqual(aut, bytearray([0x41, 0x42])) + + aut = Convert.stringToArray("ABCD", 6) + self.assertEqual(aut, bytearray([0x41, 0x42, 0x43, 0x44, 0x00, 0x00])) + + def testIntToArray(self): + test_metas = [ + { + 'value': 65536, # not a short (1 over max) + 'length': 8, + # good_bytes: b'\x00\x00\x00\x00\x00\x01\x00\x00' + }, + { + 'value': 65536, + 'length': 4, + # good_bytes: b'\x00\x01\x00\x00', + }, + { + 'value': 281470681743360, # 65535 << 32 + 'length': 8, + # 'good_bytes': b'\x00\x00\xff\xff\x00\x00\x00\x00', + } + ] + for test_meta in test_metas: + value = test_meta['value'] + length = test_meta['length'] + good_bytes = struct.pack(">{}s".format(length), + value.to_bytes(length, 'big')) + self.assertEqual(Convert.intToArray(value, length), + good_bytes) + + def testIntToArrayFail(self): + test_metas = [ + { + 'value': 65536, # not a short (1 over max) + 'length': 2, + # good_bytes: b'\x00\x00\x00\x00\x00\x01\x00\x00' + }, + { + 'value': 281470681743360, # 65535 << 32 + 'length': 4, + # 'good_bytes': b'\x00\x00\xff\xff\x00\x00\x00\x00', + } + ] + for test_meta in test_metas: + value = test_meta['value'] + length = test_meta['length'] + with self.assertRaises(ValueError): + Convert.intToArray(value, length) + + def testSpaceDecode(self): + byte6 = False + space = 0x00 + + (byte6, space) = Convert.spaceDecode(0xF8) + self.assertEqual(space, 0xF8) + self.assertTrue(byte6) + + (byte6, space) = Convert.spaceDecode(0xFF) + self.assertEqual(space, 0x03) + self.assertFalse(byte6) + + (byte6, space) = Convert.spaceDecode(0xFD) + self.assertEqual(space, 0x01) + self.assertFalse(byte6) diff --git a/tests/test_memoryservice.py b/tests/test_memoryservice.py index 0705214..8033d20 100644 --- a/tests/test_memoryservice.py +++ b/tests/test_memoryservice.py @@ -6,6 +6,7 @@ from logging import getLogger +from openlcb.convert import Convert from openlcb.physicallayer import PhysicalLayer if __name__ == "__main__": logger = getLogger(__file__) @@ -80,15 +81,6 @@ def setUp(self): ) self.mService = MemoryService(self.dService) - def testReturnCyrillicStrings(self): - # See also testReturnCyrillicStrings in test_snip - # If you have characters specific to UTF-8 (either in code or comment) - # add the following as the 1st or 2nd line of the py file: - # -*- coding: utf-8 -*- - data = bytearray([0xd0, 0x94, 0xd0, 0xbc, 0xd0, 0xb8, 0xd1, 0x82, 0xd1, 0x80, 0xd0, 0xb8, 0xd0, 0xb9]) # Cyrillic spelling of the name Dmitry (7 characters becomes 14 bytes) # noqa: E501 - self.assertEqual(self.mService.arrayToString(data, len(data)), "Дмитрий") # Cyrillic spelling of the name Dmitry. This string should appear as 7 Cyrillic characters like Cyrillic-demo-Dmitry.png in doc (14 bytes in a hex editor), otherwise your editor does not support utf-8 and editing this file with it could break it. # noqa:E501 - # TODO: Russian version is Дми́трий according to . See Cyrillic-demo-Dmitry-Russian.png in doc. # noqa:E501 - def testSingleRead(self): memMemo = MemoryReadMemo(NodeID(123), 64, 0xFD, 0, self.callbackR, self.callbackR) @@ -191,93 +183,6 @@ def testMultipleRead(self): self.assertEqual(len(LinkMockLayer.sentMessages), 5) # read reply datagram reply sent and next datagram sent # noqa: E501 self.assertEqual(len(self.returnedMemoryReadMemo), 2) # memory read returned # noqa: E501 - def testArrayToString(self): - sut = MemoryService.arrayToString(bytearray([0x41, 0x42, 0x43, 0x44]), 4) # noqa:E501 - self.assertEqual(sut, "ABCD") - - sut = MemoryService.arrayToString(bytearray([0x41, 0x42, 0, 0x44]), 4) - self.assertEqual(sut, "AB") - - sut = MemoryService.arrayToString(bytearray([0x41, 0x42, 0x43, 0x44]), 2) # noqa:E501 - self.assertEqual(sut, "AB") - - sut = MemoryService.arrayToString(bytearray([0x41, 0x42, 0x43, 0]), 4) - self.assertEqual(sut, "ABC") - - sut = MemoryService.arrayToString(bytearray([0x41, 0x42, 0x31, 0x32]), 8) # noqa:E501 - self.assertEqual(sut, "AB12") - - def testStringToArray(self): - aut = MemoryService.stringToArray("ABCD", 4) - self.assertEqual(aut, bytearray([0x41, 0x42, 0x43, 0x44])) - - aut = MemoryService.stringToArray("ABCD", 2) - self.assertEqual(aut, bytearray([0x41, 0x42])) - - aut = MemoryService.stringToArray("ABCD", 6) - self.assertEqual(aut, bytearray([0x41, 0x42, 0x43, 0x44, 0x00, 0x00])) - - def testIntToArray(self): - test_metas = [ - { - 'value': 65536, # not a short (1 over max) - 'length': 8, - # good_bytes: b'\x00\x00\x00\x00\x00\x01\x00\x00' - }, - { - 'value': 65536, - 'length': 4, - # good_bytes: b'\x00\x01\x00\x00', - }, - { - 'value': 281470681743360, # 65535 << 32 - 'length': 8, - # 'good_bytes': b'\x00\x00\xff\xff\x00\x00\x00\x00', - } - ] - for test_meta in test_metas: - value = test_meta['value'] - length = test_meta['length'] - good_bytes = struct.pack(">{}s".format(length), - value.to_bytes(length, 'big')) - self.assertEqual(MemoryService.intToArray(value, length), - good_bytes) - - def testIntToArrayFail(self): - test_metas = [ - { - 'value': 65536, # not a short (1 over max) - 'length': 2, - # good_bytes: b'\x00\x00\x00\x00\x00\x01\x00\x00' - }, - { - 'value': 281470681743360, # 65535 << 32 - 'length': 4, - # 'good_bytes': b'\x00\x00\xff\xff\x00\x00\x00\x00', - } - ] - for test_meta in test_metas: - value = test_meta['value'] - length = test_meta['length'] - with self.assertRaises(ValueError): - MemoryService.intToArray(value, length) - - def testSpaceDecode(self): - byte6 = False - space = 0x00 - - (byte6, space) = self.mService.spaceDecode(0xF8) - self.assertEqual(space, 0xF8) - self.assertTrue(byte6) - - (byte6, space) = self.mService.spaceDecode(0xFF) - self.assertEqual(space, 0x03) - self.assertFalse(byte6) - - (byte6, space) = self.mService.spaceDecode(0xFD) - self.assertEqual(space, 0x01) - self.assertFalse(byte6) - if __name__ == '__main__': unittest.main() From 10fe25a6e6274eee85aa2e8bdc0a335cfc640004 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 18 Mar 2026 18:29:24 -0400 Subject: [PATCH 02/34] Handle CDIVar size as max for "string" and add tests for edge cases. --- openlcb/cdivar.py | 32 +++++-- python-openlcb.code-workspace | 1 + tests/test_cdivar.py | 13 +-- tests/test_cdivar_edge_cases.py | 151 ++++++++++++++++++++++++++++++++ 4 files changed, 186 insertions(+), 11 deletions(-) create mode 100644 tests/test_cdivar_edge_cases.py diff --git a/openlcb/cdivar.py b/openlcb/cdivar.py index 8025905..9aea7ef 100644 --- a/openlcb/cdivar.py +++ b/openlcb/cdivar.py @@ -1,15 +1,17 @@ import struct -from openlcb import emit_cast +from logging import getLogger from typing import List, Type, Union +from openlcb import emit_cast from openlcb.eventid import EventID from openlcb.openlcbaction import OpenLCBAction +logger = getLogger(__name__) NUM_TYPES = {'int': int, 'float': float} # type: dict[str, Type] -# Assumes "IEEE" in LCC CDI Standard means IEEE 754-2008: +# Assumes "IEEE" in OpenLCB CDI Standard means IEEE 754-2008: FLOAT_MAXIMUMS = {16: 65504.0, 32: 3.40e38, 64: 1.80e308} # type: dict[int, float] # noqa: E501 CLASSNAME_TYPES = {'int': int, 'float': float, 'string': str, 'blob': bytearray, 'eventid': EventID, @@ -45,7 +47,7 @@ class CDIVar: (for className == "float"). signed (bool): Whether the value is signed (False unless min is negative). Defaults to True. - See LCC "Configuration Description Information" Standard. + See OpenLCB "Configuration Description Information" Standard. _data (bytes): The value read from the device or ready to write. Only None if not read yet, otherwise length must be .size. @@ -139,8 +141,13 @@ def stringToData(self, value: str) -> bytes: return value.encode("utf-8") def setString(self, value: str): - self.data = self.stringToData(value) - self.size = len(self.data) + # self.data = self.stringToData(value) + # self.size = len(self.data) + # assert self.className == "string" + encoded = value.encode("utf-8") + assert self.size is not None + assert len(encoded) + 1 <= self.size # size is max *only* if "string" + self.data = encoded + b"\x00" # null-terminated for OpenLCB network def dataToInt(self, data) -> Union[int, None]: assert self.className == "int" @@ -173,4 +180,17 @@ def dataToString(self, data) -> Union[str, None]: return data.decode("utf-8") def getString(self) -> Union[str, None]: - return self.dataToString(self.data) + # return self.dataToString(self.data) + if self.data is None or len(self.data) == 0: + return None + # Return content up to (but not including) first null + null_pos = self.data.find(b"\x00") + if null_pos == -1: + logger.error(f"No null terminator in {repr(self.data)}") + content = self.data + else: + content = self.data[:null_pos] + # try: + return content.decode("utf-8") + # except UnicodeDecodeError: + # return None # or raise diff --git a/python-openlcb.code-workspace b/python-openlcb.code-workspace index 160c158..a7c562d 100644 --- a/python-openlcb.code-workspace +++ b/python-openlcb.code-workspace @@ -75,6 +75,7 @@ "offvalue", "onvalue", "openlcb", + "openlcbaction", "openlcbnetwork", "padx", "pady", diff --git a/tests/test_cdivar.py b/tests/test_cdivar.py index 22af185..51682dc 100644 --- a/tests/test_cdivar.py +++ b/tests/test_cdivar.py @@ -18,12 +18,15 @@ def test_initialization_valid(self): self.assertEqual(cdivar_float.max, 100.0) self.assertEqual(cdivar_float.size, 4) + maxSize = 100 cdivar_string = CDIVar(className='string', - _default=bytearray(b'Hello')) + _default=bytearray(b'Hello'), + _size=maxSize) self.assertEqual(cdivar_string.className, 'string') self.assertEqual(cdivar_string.default, bytearray(b'Hello')) assert cdivar_string.default is not None - self.assertEqual(cdivar_string.size, len(cdivar_string.default)) + # self.assertEqual(cdivar_string.size, len(cdivar_string.default)) + self.assertEqual(cdivar_string.size, maxSize) def test_initialization_invalid_classname(self): with self.assertRaises(AssertionError): @@ -60,7 +63,7 @@ def test_set_get_float(self): self.assertAlmostEqual(got, 3.14, places=6) def test_set_get_string(self): - cdivar_string = CDIVar(className='string') + cdivar_string = CDIVar(className='string', _size=100) cdivar_string.setString("Hello") self.assertEqual(cdivar_string.getString(), "Hello") @@ -75,8 +78,8 @@ def test_invalid_set_float(self): cdivar_float.setFloat("not a float") # type:ignore (assertRaises) def test_invalid_set_string(self): - cdivar_string = CDIVar(className='string') - with self.assertRaises(AssertionError): + cdivar_string = CDIVar(className='string', _size=100) + with self.assertRaises(AttributeError): # number has no attribute 'encode' cdivar_string.setString(12345) # type:ignore (assertRaises) diff --git a/tests/test_cdivar_edge_cases.py b/tests/test_cdivar_edge_cases.py new file mode 100644 index 0000000..7820c43 --- /dev/null +++ b/tests/test_cdivar_edge_cases.py @@ -0,0 +1,151 @@ +from typing import List, Tuple +import unittest + +from openlcb.cdivar import CDIVar + + +class TestCDIVarNumericConversions(unittest.TestCase): + + def assertBytesEqual(self, expected_hex: List[int], actual: bytes, + msg: str = ""): + expected = bytes(expected_hex) + self.assertEqual( + expected, + actual, + (f"{msg}\n Expected: {expected.hex(' ').upper()}" + f"\n Got: {actual.hex(' ').upper()}") + ) + + # ------------------------------------------------------------------------- + # Basic signed int conversions — edge cases (4 bytes) + # ------------------------------------------------------------------------- + def test_setInt_getInt_4byte_edge_cases(self): + cases: List[Tuple[int, List[int]]] = [ + (-1, [0xFF, 0xFF, 0xFF, 0xFF]), + (-2147483648, [0x80, 0x00, 0x00, 0x00]), # INT32_MIN + (2147483647, [0x7F, 0xFF, 0xFF, 0xFF]), + (0, [0x00, 0x00, 0x00, 0x00]), + (300, [0x00, 0x00, 0x01, 0x2C]), + (0x12345678, [0x12, 0x34, 0x56, 0x78]), + ] + + for value, expected_bytes in cases: + with self.subTest(f"int {value} → bytes"): + var = CDIVar("int", _size=4, _min=-1) # signed + var.setInt(value) + assert var.data is not None + self.assertBytesEqual(expected_bytes, var.data) + + restored = var.getInt() + self.assertEqual(value, restored) + + # ------------------------------------------------------------------------- + # Smaller sizes — sign extension behavior + # ------------------------------------------------------------------------- + def test_small_int_sizes_sign_extension(self): + cases = [ + # value, size, signed, bytes, expected getInt + (-100, 2, True, [0xFF, 0x9C], -100), + (0xABCD, 2, False, [0xAB, 0xCD], 0xABCD), + (-128, 4, True, [0xFF, 0xFF, 0xFF, 0x80], -128), + (0x5A, 1, False, [0x5A], 0x5A), + ] + + for val, size, signed, exp_bytes, exp_restored in cases: + with self.subTest(f"{val} @ {size} bytes signed={signed}"): + var = CDIVar("int", _size=size, _min=-1 if signed else 0) + var.setInt(val) + assert var.data is not None + self.assertBytesEqual(exp_bytes, var.data) + + restored = var.getInt() + self.assertEqual(exp_restored, restored) + + # ------------------------------------------------------------------------- + # Strict IEEE 754 binary16 (half-precision) bit-exact tests + # ------------------------------------------------------------------------- + def test_float16_strict_bit_exact(self): + cases = [ # noqa: E501 + # value expected [high, low] description + (0.0, [0x00, 0x00], "+0.0"), + (5.9604644775390625e-8, [0x00, 0x01], "smallest positive subnormal"), # noqa: E501 + (-5.9604644775390625e-8, [0x80, 0x01], "smallest negative subnormal"), # noqa: E501 + (6.103515625e-5, [0x04, 0x00], "smallest positive normal"), # noqa: E501 + (-6.103515625e-5, [0x84, 0x00], "smallest negative normal"), # noqa: E501 + (1.0, [0x3C, 0x00], "1.0 exact"), + (-1.0, [0xBC, 0x00], "-1.0"), + (0.5, [0x38, 0x00], "0.5"), + (-0.5, [0xB8, 0x00], "-0.5"), + (65504.0, [0x7B, 0xFF], "max finite"), + (-65504.0, [0xFB, 0xFF], "max negative finite"), # noqa: E501 + (float("inf"), [0x7C, 0x00], "+Inf"), + (float("-inf"), [0xFC, 0x00], "-Inf"), + # (float("nan"), [0x7E, 0x00], "canonical quiet NaN"), # noqa: E501 + # (65536.0, [0x7C, 0x00], "overflow → +Inf"), # noqa: E501 + # (1.00048828125, [0x3C, 0x01], "ties-to-even example"), # noqa: E501 + (float("nan"), [0x7E, 0x00], "canonical quiet NaN"), # noqa: E501 + # 65536.0 removed — Python struct raises OverflowError (expected) + # (1.00048828125, [0x3C, 0x00], "ties-to-even rounds to even (down in this case)"), # noqa: E501 + # ^ becomes 1.0 due to float16 precision, so commented + (1.0009765625, [0x3C, 0x01], "1 + 2⁻¹⁰ = exact in float16"), # noqa: E501 + # (1.00048828125 + 1e-12, [0x3C, 0x01], "slightly above midpoint → rounds up"), # noqa: E501 + # ^ AssertionError: 1.000488281251 != 1.0009765625 : Round-trip mismatch: 1.000488281251 → 1.0009765625 # noqa: E501 + # due to float16 precision + ] + + for val, expected, message in cases: + with self.subTest(f"float16 {val}"): + var = CDIVar("float", _size=2) + var.setFloat(val) + assert var.data is not None + self.assertBytesEqual(expected, var.data, + f"setFloat 16 ({val}) {message} failed") # noqa: E501 + + # round-trip check + restored = var.getFloat() + assert restored is not None + if val != val: # NaN + self.assertTrue(restored != restored) + elif abs(val) == float("inf"): + self.assertTrue( + abs(restored) == float("inf") and (restored > 0) == (val > 0), # noqa: E501 + f"setFloat 16 {message} failed" + ) + else: + # For representable values → should be bit-exact round-trip + self.assertEqual( + val, restored, + f"Round-trip mismatch: {val} → {restored}" + ) + + # ------------------------------------------------------------------------- + # Basic null-terminated string behavior (modified methods) + # ------------------------------------------------------------------------- + def test_string_null_terminated(self): + cases = [ + ("hello", b"hello\x00"), + ("", b"\x00"), + ("café π", "café π".encode("utf-8") + b"\x00"), + ] + + for s, expected_bytes in cases: + with self.subTest(f"setString({s!r})"): + var = CDIVar("string", _size=100) + var.setString(s) + self.assertEqual(expected_bytes, var.data) + + restored = var.getString() + self.assertEqual(s, restored) + + # Extra data after null is ignored + var = CDIVar("string") + var.data = b"test\x00junk" + self.assertEqual("test", var.getString()) + + # No null → whole content + var.data = b"no-null-here" + self.assertEqual("no-null-here", var.getString()) + + +if __name__ == "__main__": + unittest.main(verbosity=2) \ No newline at end of file From 9b29e37e1bbe36a0355bf02f54989aaf5c2e3e14 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:46:09 -0400 Subject: [PATCH 03/34] Fix: Handle "." at end of nodeid range prefix. --- openlcb/conventions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openlcb/conventions.py b/openlcb/conventions.py index bd2af5c..842cf97 100644 --- a/openlcb/conventions.py +++ b/openlcb/conventions.py @@ -248,6 +248,7 @@ def generate_node_id_str(id_range_prefix: str, increment: bool = False) -> str: lastParts = [f"{p:02X}" for p in generate_last_three_octets(increment=increment)] # noqa: E501 assert len(lastParts) == 3 + id_range_prefix = id_range_prefix.rstrip(".") prefixParts = id_range_prefix.split(".") if len(prefixParts) < 3: raise ValueError( From adccc26dcce026f2e46cd1702ce35069874d09ee Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:47:17 -0400 Subject: [PATCH 04/34] Fix: Handle memo using new OO-defined onStatusMemo (formerly _onElement). Add memoryService property so client code can register handlers. --- examples/tkexamples/cdiform.py | 5 +++-- openlcb/openlcbnetwork.py | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/examples/tkexamples/cdiform.py b/examples/tkexamples/cdiform.py index b6702b5..ce9ba7f 100644 --- a/examples/tkexamples/cdiform.py +++ b/examples/tkexamples/cdiform.py @@ -226,8 +226,9 @@ def onStartDownload(self): def onStatusMemo(self, cm: CDIMemo) -> bool: """Handler for incoming CDI tag - Use this for callback in downloadCDI, which sets parser - (_dataProcessor)'s _onElement. + Use this for callback in downloadCDI + (onStatusMemo replaces _dataProcessor's _onElement + formerly set by downloadCDI). Args: cm (CDIMemo): Document parsing state info diff --git a/openlcb/openlcbnetwork.py b/openlcb/openlcbnetwork.py index 28bd45a..c137437 100644 --- a/openlcb/openlcbnetwork.py +++ b/openlcb/openlcbnetwork.py @@ -93,6 +93,10 @@ def __init__(self, localNodeID: Union[str, bytearray, int, NodeID]): self._memoryService = MemoryService(self._datagramService) self._dataProcessor: XMLDataProcessor = None + @property + def memoryService(self): + return self._memoryService + def setConnectHandler(self, handler: Callable[[CDIMemo], None]): """Deprecated in favor of a Message handler, Since it is the socket loop's responsibility to call @@ -292,8 +296,8 @@ def _listen(self): cm = CDIMemo() cm.error = formatted_ex(ex) cm.done = True # stop progress in gui/other main thread - if self._dataProcessor._onElement: - self._dataProcessor._onElement(cm) + if self._dataProcessor.onStatusMemo: + self._dataProcessor.onStatusMemo(cm) raise # re-raise since incomplete (prevent done OK state) finally: self.physicalLayer.physicalLayerDown() # Link_Layer_Down, setState From 472549776758f7d1706758eac291cb3c35fa5246 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:20:30 -0400 Subject: [PATCH 05/34] Enforce onStartDownload via OO. --- examples/examples_gui.py | 1 - examples/tkexamples/cdiform.py | 4 ++-- openlcb/openlcbnetwork.py | 2 +- openlcb/xmldataprocessor.py | 9 +++++++++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/examples/examples_gui.py b/examples/examples_gui.py index fab718e..a38338e 100644 --- a/examples/examples_gui.py +++ b/examples/examples_gui.py @@ -632,7 +632,6 @@ def downloadCDI(self, farNodeID: str): self.setStatus("Downloading CDI...") assert self.cdi_form is not None assert self.network is not None - self.cdi_form.onStartDownload() try: self.network.download(farNodeID, MemorySpace.CDI, self.cdi_form) diff --git a/examples/tkexamples/cdiform.py b/examples/tkexamples/cdiform.py index ce9ba7f..09fce4f 100644 --- a/examples/tkexamples/cdiform.py +++ b/examples/tkexamples/cdiform.py @@ -221,8 +221,8 @@ def getStatus(self): def onStartDownload(self): """Initialize variables used by element handler(s).""" - self.onStart() - self._resetTree() + XMLDataProcessor.onStartDownload(self) + # TODO: clear tree? def onStatusMemo(self, cm: CDIMemo) -> bool: """Handler for incoming CDI tag diff --git a/openlcb/openlcbnetwork.py b/openlcb/openlcbnetwork.py index c137437..9a362cc 100644 --- a/openlcb/openlcbnetwork.py +++ b/openlcb/openlcbnetwork.py @@ -156,6 +156,7 @@ def _startMemoryRead(self, farNodeID: Union[NodeID, int, str, bytearray]): """ # read 64 bytes from the CDI space starting at address zero assert isinstance(self._dataProcessor.space, MemorySpace) + self._dataProcessor.onStartDownload() memMemo = MemoryReadMemo(NodeID(farNodeID), 64, self._dataProcessor.space.value, 0, # incremented on _memoryReadSuccess @@ -193,7 +194,6 @@ def download(self, farNodeID: str, space: MemorySpace, assert isinstance(space, MemorySpace) self._dataProcessor = dataProcessor self._dataProcessor._space = space - self._dataProcessor._stringTerminated = False self._startMemoryRead(farNodeID) # ^ Following this, _memoryReadSuccess callback will diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 539308a..02acfc0 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -181,6 +181,15 @@ def onPopScope(self, cm: CDIMemo) -> bool: """ return False + def onStartDownload(self): + """Initialize variables used by element handler(s). + If subclass is a GUI, reimplement this to reset GUI, + but also call onStart or super().onStartDownload(). + """ + self._stringTerminated = False + self._resetTree() + self.onStart() + def onStart(self): # self._cdi_offset = 0 # Instead see memo.address (which is # incremented on _memoryReadSuccess or custom memory read From ae1d1cfdb06a16e91831066db92d63a894d1edd5 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 22 Apr 2026 18:35:05 -0400 Subject: [PATCH 06/34] Add high-level (size-aware if set manually) DataProcessor progress. Add a type hint. Reduce logging (use info level for memory read). --- openlcb/cdimemo.py | 4 ++++ openlcb/dataprocessor.py | 5 ++++- openlcb/memoryservice.py | 3 ++- openlcb/xmldataprocessor.py | 30 +++++++++++++++++++++++++++++- python-openlcb.code-workspace | 2 ++ 5 files changed, 41 insertions(+), 3 deletions(-) diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index 78ee35a..3cc8b15 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -62,6 +62,10 @@ def __init__(self, tag: Union[str, None] = None, self.address = None # type: int|None self.cdivar = None # type: CDIVar|None self.children = [] # type: List[CDIMemo] + # Set by DataProcessor such as XMLDataProcessor: + self.progress_ratio = None # type: float|None + self.progress_count = None # type: int|None + self.expected_size = None # type: int|None def getTag(self): if self.element is None: diff --git a/openlcb/dataprocessor.py b/openlcb/dataprocessor.py index 75a8f96..d9fab76 100644 --- a/openlcb/dataprocessor.py +++ b/openlcb/dataprocessor.py @@ -12,4 +12,7 @@ class DataProcessor: Superclass for data listeners. """ def __init__(self): - pass + # Members used to construct space memo such as CDIMemo: + self.progress_ratio = None # type: float|None + self.progress_count = None # type: int|None + self.expected_size = None # type: int|None diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index c61e51b..f4f7024 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -328,7 +328,8 @@ def requestMemoryWrite(self, memo: MemoryWriteMemo): dgWriteMemo = DatagramWriteMemo(memo.nodeID, data) self.service.sendDatagram(dgWriteMemo) - def requestSpaceLength(self, space, nodeID, callback): + def requestSpaceLength(self, space: int, nodeID: NodeID, + callback: Callable[[int], None]): '''Request the length of a specific memory space from a remote node. Args: diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 02acfc0..860b616 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -199,6 +199,7 @@ def onStart(self): "A previous downloadCDI operation is in progress" " or failed (Set _data to None first if failed)") self._data = bytearray() + self.progress_count = 0 def onStop(self): self._format = DataFormat.EOF # no data expected @@ -213,11 +214,22 @@ def _fireStatus(self, status, if callback is None: callback = self.onStatusMemo if callback: - print("OpenLCBNetwork callback_msg({})".format(repr(status))) + logger.info("OpenLCBNetwork callback_msg({})".format(repr(status))) callback(CDIMemo(status=status)) else: logger.warning("No callback, but set status: {}".format(status)) + def _fireStatusMemo(self, statusMemo, + callback: Union[Callable[[CDIMemo], bool], None] = None): # noqa: E501 + """Fire status handlers with the given status.""" + if callback is None: + callback = self.onStatusMemo + if callback: + logger.info(f"OpenLCBNetwork callback_msg({statusMemo})") + callback(statusMemo) + else: + logger.warning(f"No callback, but set status: {statusMemo}") + def _feedNext(self, memo: MemoryReadMemo): """Handle partial CDI XML (any packet except last) The last packet is not yet reached, so don't parse (but @@ -229,9 +241,14 @@ def _feedNext(self, memo: MemoryReadMemo): """ assert self._data is not None self._data += memo.data + self.progress_count = len(self._data) partial_str = memo.data.decode("utf-8") if self._realtime: self._parser.feed(partial_str) # may call startElement/endElement + cm = CDIMemo() + cm.progress_count = self.progress_count + cm.expected_size = self.expected_size + self.onStatusMemo(cm) def _feedLast(self, memo: MemoryReadMemo): """Handle end of CDI XML (last packet) @@ -255,6 +272,13 @@ def _feedLast(self, memo: MemoryReadMemo): if null_i > -1: terminate_i = min(null_i, terminate_i) partial_str = memo.data[:terminate_i].decode("utf-8") + assert self.progress_count is not None + self.progress_count += terminate_i + cm = CDIMemo() + cm.done = True # 'done' and not 'error' means got all + cm.progress_count = self.progress_count + cm.expected_size = self.expected_size + self.onStatusMemo(cm) else: # *not* realtime (but got to end, so parse all at once) cdiString = "" @@ -263,6 +287,9 @@ def _feedLast(self, memo: MemoryReadMemo): if null_i > -1: terminate_i = min(null_i, terminate_i) cdiString = self._data[:terminate_i].decode("utf-8") + assert self.progress_count is not None + self.progress_count += terminate_i + # print (cdiString) # self.parse(cdiString) # no such method # self._parser.parse(cdiString) # urllib.error.URLError @@ -273,6 +300,7 @@ def _feedLast(self, memo: MemoryReadMemo): # self._fireStatus("Done loading CDI.") cm = CDIMemo() cm.done = True # 'done' and not 'error' means got all + cm.progress_count = self.progress_count self.onStatusMemo(cm) if self._realtime: self._parser.feed(partial_str) # may call startElement/endElement diff --git a/python-openlcb.code-workspace b/python-openlcb.code-workspace index a7c562d..bf30c55 100644 --- a/python-openlcb.code-workspace +++ b/python-openlcb.code-workspace @@ -41,6 +41,7 @@ "cdiform", "cdimemo", "cdivar", + "cdivars", "columnspan", "controlframe", "datagram", @@ -108,6 +109,7 @@ "usbmodem", "WASI", "winnative", + "wraplength", "xmldataprocessor", "xscrollcommand", "zeroconf" From 7192c264e753fbbe2d542f01bc80de510d2c2af5 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 22 Apr 2026 19:55:07 -0400 Subject: [PATCH 07/34] Fix: Provide size to CDIVar constructor. Add CDIVar serialization and name (Set externally optionally). --- openlcb/cdimemo.py | 34 +++++++++++++++++++++++++++------- openlcb/cdivar.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index 3cc8b15..2d4a75e 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -156,17 +156,37 @@ def toCDIVar(self): """Create a CDIVar from descriptors (child elements of self). See LCC "Configuration Description Information" Standard. """ - result = CDIVar(self.tag) + # result = CDIVar(self.tag) assert (self.tag is not None) and (self.tag.strip()) - result.className = self.tag.lower() + className = self.tag.lower() + result_floatFormat = None if self.element: - result.floatFormat = self.element.attrib.get('floatFormat') + result_floatFormat = self.element.attrib.get('floatFormat') this_t = NUM_TYPES.get(self.tag) if self.tag else None + result_min = None + result_max = None + result_default = None + result_size = self.getSize() if this_t is not None: - result.min = self.getChildContentN("min", result.className) - result.max = self.getChildContentN("max", result.className) - result.default = self.getChildContentN("default", result.className) - result.size = self.getSize() + result_min = self.getChildContentN("min", className) + result_max = self.getChildContentN("max", className) + default_n = self.getChildContentN("default", className) + if default_n is not None: + default_var = CDIVar(className, _size=result_size) + if isinstance(default_n, int): + assert self.tag == "int" + default_var.setInt(default_n) + else: + assert self.tag == "float" + default_var.setFloat(default_n) + assert default_var.data is not None + result_default = bytearray(default_var.data) + # Size must be gotten ahead of time since CDIVar constructor + # enforces size: + result = CDIVar(self.tag, _min=result_min, _max=result_max, + _size=result_size, _default=result_default) + result.floatFormat = result_floatFormat + result.name = self.getChildContent("name") if result.className == "int": if result.min is None: diff --git a/openlcb/cdivar.py b/openlcb/cdivar.py index 9aea7ef..0474ed3 100644 --- a/openlcb/cdivar.py +++ b/openlcb/cdivar.py @@ -1,4 +1,6 @@ +import base64 +from collections import OrderedDict import struct from logging import getLogger @@ -61,6 +63,7 @@ def __init__(self, className, _min=None, _max=None, assert className, f"Expected {CLASSNAME_TYPES.keys()} got {className}" assert className in CLASSNAME_TYPES, \ f"Expected {list(CLASSNAME_TYPES.keys())} got {className}" + self.name = None # type: str|None self.className = className # type: str self.data = None # type: bytes|None self.min = _min # type: int|float|None @@ -85,9 +88,11 @@ def standardSizes(self) -> Union[List[int], None]: def assertNumberFormat(self, assertWhat=""): if self.className == "int": - assert self.size in (1, 2, 4, 8) + assert self.size in (1, 2, 4, 8), \ + f"Expected size (1, 2, 4, 8) for int, got {self.size}" elif self.className == "float": - assert self.size in (2, 4, 8) + assert self.size in (2, 4, 8), \ + f"Expected size (2, 4, 8) for float, got {self.size}" else: if not assertWhat: assertWhat = f"Expected float/int size {STANDARD_SIZES}" @@ -124,6 +129,25 @@ def intToData(self, value: int) -> bytes: assert isinstance(value, int) return struct.pack(self.packFormat(), value) + def getSerializable(self): + """Get a value in the corresponding Python type""" + if self.className == "int": + return self.getInt() + elif self.className == "float": + return self.getFloat() + elif self.className == "string": + return self.getString() + assert self.className in ("blob", "eventid", "action") + return base64.b64encode(self.data) + + def getDict(self, add_name=True): + result = OrderedDict() + if add_name and self.name: + result['name'] = self.name + result['className'] = self.className + result['value'] = self.getSerializable() + return result + def setInt(self, value: int): self.data = self.intToData(value) From 921f518523925ab66607cb752189f6abf4ea8d64 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 22 Apr 2026 20:06:12 -0400 Subject: [PATCH 08/34] Save the memo tree in XMLDataProcessor (formerly had to be collected by callbacks if necessary). --- openlcb/xmldataprocessor.py | 44 +++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 860b616..6df7dcd 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -105,6 +105,8 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): def __init__(self, linkLayer: CanLink, space: MemorySpace): self.canLink: CanLink = linkLayer caches_dir = SysDirs.Cache + self._root_memos = None # type: list[CDIMemo]|None + self._root_memo = None # type: CDIMemo|None self._space: Union[MemorySpace, None] = None self._openEl: Union[ET.Element, None] = None self._top_tag = "cdi" # cdi or fdi (detected in startElement) @@ -136,6 +138,42 @@ def __init__(self, linkLayer: CanLink, space: MemorySpace): # endregion ContentHandler self.acdi = False + def getRootMemo(self): + """Get the root memo object if any. + This should only be called after the entire file is parsed such + as when cm.done is True in onStatusMemo(cm) callback. Set + callback manually if necessary and if using realtime parsing + (_feed) mode. + """ + if not self._root_memos: + return None + if len(self._root_memos) > 1: + summaries = [] + cdi_roots = [] + tag = None + for memo in self._root_memos: + tag = memo.getTag() + if tag is not None: + tag = tag.lower() + summaries.append(memo.getTag()) + if tag in ("cdi", "fdi"): + cdi_roots.append(memo) + if len(cdi_roots) == 1: + return cdi_roots[0] + if tag not in ("cdi", "fdi"): + logger.warning( + f"Got more than one XML root: {summaries};" + " expected cdi/fdi") + else: + logger.warning(f"Got more than one XML root: {summaries}") + return self._root_memos[-1] + tag = self._root_memos[0].getTag() + if tag is not None: + tag = tag.lower() + if tag not in ("cdi", "fdi"): + logger.warning(f"Only XML root is {repr(tag)} not cdi/fdi") + return self._root_memos[0] + def setSpace(self, space: MemorySpace): self._space = space self._format = format_of_space(space) @@ -200,6 +238,8 @@ def onStart(self): " or failed (Set _data to None first if failed)") self._data = bytearray() self.progress_count = 0 + self._root_memos = [] # list of roots + self._root_memo = None def onStop(self): self._format = DataFormat.EOF # no data expected @@ -379,6 +419,10 @@ def startElement(self, name: str, cm.address = self._tmp_address # May be None if after /segment self.onPushScope(cm) + if len(self._tag_stack) < 1: + self._root_memos.append(cm) + if cm.tag == "cdi": + self._root_memo = cm # self._callback_msg( # "loaded: {}{}".format(tab, ET.tostring(el, encoding="unicode"))) From db12c3dee52948cacdc1a434fd146c3aa9e6de7f Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:01:19 -0400 Subject: [PATCH 09/34] Add load feature (Load cached CDI XML file). Make caching configurable. Add fromNumber. Change cache_cdi_path to classmethod. Add fromNumber to MemorySpace enum. Add force_end option in _memoryReadSuccess (reserved for future use). --- openlcb/dataprocessor.py | 4 ++ openlcb/memoryservice.py | 11 +++++ openlcb/openlcbnetwork.py | 5 +- openlcb/xmldataprocessor.py | 96 ++++++++++++++++++++++++++++++++++--- 4 files changed, 107 insertions(+), 9 deletions(-) diff --git a/openlcb/dataprocessor.py b/openlcb/dataprocessor.py index d9fab76..d21e99a 100644 --- a/openlcb/dataprocessor.py +++ b/openlcb/dataprocessor.py @@ -10,8 +10,12 @@ class DataFormat(Enum): class DataProcessor: """Collect & process consecutive data from each incoming MemoryReadMemo. Superclass for data listeners. + + Attributes: + enable_cache (bool): Defaults to False (May differ in subclass). """ def __init__(self): + self.enable_cache = False # type: bool # Members used to construct space memo such as CDIMemo: self.progress_ratio = None # type: float|None self.progress_count = None # type: int|None diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index f4f7024..698bf8d 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -65,6 +65,17 @@ class MemorySpace(Enum): All = 0xFE Configuration = 0xFD + @classmethod + def fromNumber(cls, num: int): + """Return the MemorySpace member with the given numeric value, + or None if no match is found. + """ + assert isinstance(num, int) + for member in cls: + if member.value == num: + return member + return None + class MemoryReadMemo: """Memo carries request and reply. diff --git a/openlcb/openlcbnetwork.py b/openlcb/openlcbnetwork.py index 9a362cc..fd70f72 100644 --- a/openlcb/openlcbnetwork.py +++ b/openlcb/openlcbnetwork.py @@ -363,7 +363,7 @@ def _fireStatus(self, status, else: logger.warning("No callback, but set status: {}".format(status)) - def _memoryReadSuccess(self, memo: MemoryReadMemo): + def _memoryReadSuccess(self, memo: MemoryReadMemo, force_end=False): """Handle a successful read Invoked when the memory read successfully returns, this queues a new read until the entire CDI has been @@ -373,7 +373,8 @@ def _memoryReadSuccess(self, memo: MemoryReadMemo): memo (MemoryReadMemo): Successful MemoryReadMemo """ # print("successful memory read: {}".format(memo.data)) - if len(memo.data) == 64 and 0 not in memo.data: # *not* last chunk + if (not force_end) and (len(memo.data) == 64 and 0 not in memo.data): + # *not* last chunk self._dataProcessor._stringTerminated = False if self._dataProcessor.format != DataFormat.EOF: # save content diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 6df7dcd..b1c304b 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -59,13 +59,15 @@ def attrs_to_ordered(attrs: xml.sax.xmlreader.AttributesImpl): return od -def format_of_space(space): +def format_of_space(space, unknown_raises=True): assert isinstance(space, MemorySpace) if space == MemorySpace.CDI: return DataFormat.XML elif space == MemorySpace.FDI: return DataFormat.XML - raise NotImplementedError(emit_cast(space)) + if unknown_raises: + raise NotImplementedError(emit_cast(space)) + return None class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): @@ -101,16 +103,19 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): of start tags). """ XML_TOP_TAGS = ("cdi", "fdi") + DEFAULT_CACHES_DIR = SysDirs.Cache + DEFAULT_CACHE_DIR = os.path.join(DEFAULT_CACHES_DIR, "python-openlcb") def __init__(self, linkLayer: CanLink, space: MemorySpace): self.canLink: CanLink = linkLayer - caches_dir = SysDirs.Cache + # caches_dir = SysDirs.Cache self._root_memos = None # type: list[CDIMemo]|None self._root_memo = None # type: CDIMemo|None self._space: Union[MemorySpace, None] = None self._openEl: Union[ET.Element, None] = None self._top_tag = "cdi" # cdi or fdi (detected in startElement) - self._myCacheDir = os.path.join(caches_dir, "python-openlcb") + # self._myCacheDir = os.path.join(caches_dir, "python-openlcb") + self._myCacheDir = XMLDataProcessor.DEFAULT_CACHE_DIR self._tmp_space = None # type: int|None self._tmp_address = None # type: int|None assert isinstance(space, MemorySpace) @@ -120,6 +125,7 @@ def __init__(self, linkLayer: CanLink, space: MemorySpace): # prepare these for _callback_msg. xml.sax.ContentHandler.__init__(self) DataProcessor.__init__(self) + self.enable_cache = True self._stringTerminated = None # type: Union[bool, None] # ^ None means no read is occurring. if self._format != DataFormat.XML: @@ -197,6 +203,7 @@ def onStatusMemo(self, cm: CDIMemo) -> bool: Returns: bool: True if handled. """ + logger.warning("Default onStatusMemo ran.") return False def onPushScope(self, cm: CDIMemo) -> bool: @@ -290,13 +297,82 @@ def _feedNext(self, memo: MemoryReadMemo): cm.expected_size = self.expected_size self.onStatusMemo(cm) - def _feedLast(self, memo: MemoryReadMemo): + def load(self, node_id: NodeID, path, space: Union[MemorySpace, int], + memo: Union[MemoryReadMemo, None] = None, + format: Union[DataFormat, None] = None): + """Load instead of downloading.""" + assert not self._data + self.onStartDownload() + assert isinstance(space, (MemorySpace, int)) + if isinstance(space, int): + try_space = MemorySpace.fromNumber(space) + if try_space is not None: + space = try_space + if isinstance(space, MemorySpace): + self.setSpace(space) + else: + if format is None: + raise ValueError(f"Using device-specific space: {space}" + " but format not specified") + else: + assert isinstance(format, DataFormat) + self._format = format + self._space = space # int in device-specific case + logger.warning(f"Using device-specific space: {space}") + data = None + with open(path, "rb") as stream: + data = stream.read() # type:ignore + if self._format is DataFormat.XML: + if memo is not None: + assert isinstance(memo, MemoryReadMemo) + else: + def memoryReadSuccess(memo: MemoryReadMemo): + # See further down + print("Fallback memoryReadSuccess ran.") + pass + + def memoryReadFail(memo: MemoryReadMemo): + raise RuntimeError( + "Offline parse failure (should never happen)") + + assert data is not None + # Based on _startMemoryRead in OpenLCBNetwork: + memo = MemoryReadMemo(node_id, len(data), + self.getSpaceValue(), 0, + memoryReadFail, memoryReadSuccess) + + assert data is not None + memo.data = data # type: ignore + self._data = bytearray() # Since _feedLast adds memo.data to it + memo.size = len(data) + # based on "else" (done) case in _memoryReadSuccess + # in OpenLCBNetwork: + self._stringTerminated = True + self._feedLast(memo, enable_cache=False) + self.onStop() # sets self._format to DataFormat.EOF + else: + logger.warning(f"Custom DataFormat {self._format}" + f" (space={space}): not parsed automatically.") + + def getSpaceValue(self): + # type: () -> int|None + if self._space is None: + return None + if isinstance(self._space, MemorySpace): + return self._space.value + assert isinstance(self._space, int) + return self._space + + def _feedLast(self, memo: MemoryReadMemo, enable_cache=None): """Handle end of CDI XML (last packet) End of data, so parse (or feed if self._realtime) Args: memo (MemoryReadMemo): successful read memo containing data. + enable_cache (bool): Defaults to self.enable_cache. """ + if enable_cache is None: + enable_cache = self.enable_cache partial_str = memo.data.decode("utf-8") # save content assert self._data is not None @@ -353,8 +429,14 @@ def _feedLast(self, memo: MemoryReadMemo): print('Saved {}'.format(repr(path))) self._data = None # Ensure isn't reused for more than one doc - def cache_cdi_path(self, item_id: Union[NodeID, str]): - cdi_cache_dir = os.path.join(self._myCacheDir, "cdi") + def cache_cdi_path_scoped(self, item_id: Union[NodeID, str]): + type(self).cache_cdi_path(item_id, my_cache_dir=self._myCacheDir) + + @classmethod + def cache_cdi_path(cls, item_id: Union[NodeID, str], my_cache_dir=None): + if my_cache_dir is None: + my_cache_dir = cls.DEFAULT_CACHE_DIR + cdi_cache_dir = os.path.join(my_cache_dir, "cdi") if not os.path.isdir(cdi_cache_dir): os.makedirs(cdi_cache_dir) # TODO: add hardware name and firmware version and from SNIP to From 03ebb935e703389dbf23e14f1d0def9dcc3167e2 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Thu, 23 Apr 2026 19:01:42 -0400 Subject: [PATCH 10/34] Fix: Set size 8 for eventid. Enforce valid 'action' sizes. Add expandedTree, extractCDIVarMemos and related features. Add setData and enforce valid size(s). --- openlcb/__init__.py | 16 ++++ openlcb/cdimemo.py | 8 +- openlcb/cdivar.py | 44 ++++++++++- openlcb/xmldataprocessor.py | 150 +++++++++++++++++++++++++++++++++--- 4 files changed, 204 insertions(+), 14 deletions(-) diff --git a/openlcb/__init__.py b/openlcb/__init__.py index 0eebe1b..788de04 100644 --- a/openlcb/__init__.py +++ b/openlcb/__init__.py @@ -131,3 +131,19 @@ def from_hex_bytes(b: bytearray, start: int, stop: int, def from_all_hex_bytes(b: bytearray) -> bytearray: return from_hex_bytes(b, 0, len(b)) + + +def hr_repr(value, always_quote: bool = False) -> str: + """Represent value with double quotes + (Human-readable repr). + """ + repr_value = repr(value) + if repr_value.startswith("'") and repr_value.endswith("'"): + return '"' + repr_value[1:-1].replace('"', '\\"') + '"' + elif always_quote: + return '"' + repr_value.replace('"', '\\"') + '"' + return repr(value) + + +def d_quote(value) -> str: + return hr_repr(value, always_quote=True) diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index 2d4a75e..8112400 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -1,4 +1,5 @@ from collections import OrderedDict +import copy import json import math import xml.etree.ElementTree @@ -102,7 +103,10 @@ def getChildContent(self, tag) -> Union[str, None]: def copy(self): cm = CDIMemo() for k, v in self.__dict__.items(): - setattr(cm, k, v) + if isinstance(v, (list, dict, OrderedDict)): + setattr(cm, k, copy.deepcopy(v)) + else: + setattr(cm, k, v) return cm def getBranch(self, default=None) -> Union[str, None]: @@ -218,6 +222,8 @@ def toCDIVar(self): return result def getSize(self): + if self.tag == "eventid": + return 8 if self.element is None: return None size = self.element.attrib.get('size') diff --git a/openlcb/cdivar.py b/openlcb/cdivar.py index 0474ed3..2c667b0 100644 --- a/openlcb/cdivar.py +++ b/openlcb/cdivar.py @@ -1,10 +1,11 @@ import base64 from collections import OrderedDict +import copy import struct from logging import getLogger -from typing import List, Type, Union +from typing import Any, List, Type, Union from openlcb import emit_cast from openlcb.eventid import EventID @@ -18,6 +19,8 @@ CLASSNAME_TYPES = {'int': int, 'float': float, 'string': str, 'blob': bytearray, 'eventid': EventID, 'action': OpenLCBAction} +SIZED_CONSTRUCTION_TYPES = copy.deepcopy(CLASSNAME_TYPES) +SIZED_CONSTRUCTION_TYPES['eventid'] = bytearray SUBTYPE_FORMATS = { 'int8': "b", 'uint8': "B", 'int16': ">h", 'uint16': ">H", @@ -30,6 +33,8 @@ STANDARD_SIZES = { 'int': (1, 2, 4, 8), 'float': (2, 4, 8), + 'eventid': (8,), + 'action': (1, 2, 4, 8), } @@ -53,6 +58,8 @@ class CDIVar: _data (bytes): The value read from the device or ready to write. Only None if not read yet, otherwise length must be .size. + element (xml.etree.Element): An associated element in an XML + tree. """ TYPED_KEYS = ['min', 'max', 'default'] @@ -73,12 +80,47 @@ def __init__(self, className, _min=None, _max=None, self.max = _max # type: int|float|None self.default = _default # type: bytearray|None self.size = _size # type: int|None + self.branch_size = None # type: int|None # size including children if self.size is None: if self.default is not None: self.size = len(self.default) if self.className in ("int", "float"): self.assertNumberFormat() + elif self.className == "eventid": + if (_size is not None) and (_size != 8): + logger.error( + f'Specified eventid size="{_size}" but 8 is required.') + self.size = 8 + sizes = STANDARD_SIZES.get(self.className) + if sizes is not None: + assert self.size in sizes, \ + (f"Expected size in {sizes}" + f" for {self.className} but got {self.size}") self.floatFormat = None # type: str|None + self.address = None # type: int|None + self.element = None # type: Any|None + + def setData(self, data: Union[bytes, bytearray]): + assert isinstance(data, (bytes, bytearray)) + if isinstance(data, bytes): + data = bytearray(data) + if self.className == "eventid": + assert len(data) == 8 + elif self.className == "blob": + # FIXME: enforce blob + pass + elif self.className == "string": + assert self.size + assert len(data) <= self.size + elif self.className in SIZED_CONSTRUCTION_TYPES: + assert self.size + assert len(data) == self.size + else: + raise NotImplementedError(f"Type {self.className} not implemented") + self.data = data + + def getData(self): + return self.data def isNumber(self): return self.className in ("int", "float") diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index b1c304b..3054efc 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -6,10 +6,10 @@ from logging import getLogger from typing import Callable, List, Union -# from xml.sax.xmlreader import AttributesImpl # for type hints, for autocomplete only in this case +# from xml.sax.xmlreader import AttributesImpl # for type hints, for autocomplete only in this case # noqa:E501 import xml.sax.xmlreader # for type hints, for autocomplete only in this case -from openlcb import emit_cast +from openlcb import d_quote, emit_cast from openlcb.canbus.canlink import CanLink from openlcb.cdimemo import CDIMemo from openlcb.dataprocessor import DataFormat, DataProcessor @@ -23,6 +23,10 @@ MemorySpace, ) # from openlcb.remotenodeprocessor import RemoteNodeProcessor +from openlcb.cdivar import ( + CDIVar, + CLASSNAME_TYPES, +) if __name__ == "__main__": @@ -109,6 +113,7 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): def __init__(self, linkLayer: CanLink, space: MemorySpace): self.canLink: CanLink = linkLayer # caches_dir = SysDirs.Cache + self.expanded_root = None # type: xml.etree.ElementTree|None self._root_memos = None # type: list[CDIMemo]|None self._root_memo = None # type: CDIMemo|None self._space: Union[MemorySpace, None] = None @@ -317,7 +322,7 @@ def load(self, node_id: NodeID, path, space: Union[MemorySpace, int], else: assert isinstance(format, DataFormat) self._format = format - self._space = space # int in device-specific case + self._space = space # type:ignore # int if device-specific logger.warning(f"Using device-specific space: {space}") data = None with open(path, "rb") as stream: @@ -421,7 +426,7 @@ def _feedLast(self, memo: MemoryReadMemo, enable_cache=None): if self._realtime: self._parser.feed(partial_str) # may call startElement/endElement # memo = MemoryReadMemo(memo) - path = self.cache_cdi_path(memo.nodeID) + path = self.cacheFilePath(memo.nodeID) with open(path, 'w') as stream: if cdiString is None: cdiString = self._data.rstrip(b'\0').decode("utf-8") @@ -429,21 +434,35 @@ def _feedLast(self, memo: MemoryReadMemo, enable_cache=None): print('Saved {}'.format(repr(path))) self._data = None # Ensure isn't reused for more than one doc - def cache_cdi_path_scoped(self, item_id: Union[NodeID, str]): - type(self).cache_cdi_path(item_id, my_cache_dir=self._myCacheDir) + def cacheFilePathCustom(self, item_id: Union[NodeID, str], **kwargs): + if 'my_cache_dir' not in kwargs: + kwargs['my_cache_dir'] = self._myCacheDir + type(self).cacheFilePath(item_id, **kwargs) @classmethod - def cache_cdi_path(cls, item_id: Union[NodeID, str], my_cache_dir=None): + def cacheFilePath(cls, item_id: Union[NodeID, str], my_cache_dir=None, + subFolder="cdi", name=None, ext=".xml"): if my_cache_dir is None: my_cache_dir = cls.DEFAULT_CACHE_DIR - cdi_cache_dir = os.path.join(my_cache_dir, "cdi") + if subFolder: + cdi_cache_dir = os.path.join(my_cache_dir, subFolder) + else: + cdi_cache_dir = my_cache_dir if not os.path.isdir(cdi_cache_dir): os.makedirs(cdi_cache_dir) # TODO: add hardware name and firmware version and from SNIP to # name file to avoid cache file from a different # device/version. - item_id = str(item_id) # Convert NodeID or other - clean_name = clean_file_name(item_id.replace(":", ".")) + if not name: + item_id = str(item_id) # Convert NodeID or other + clean_name = clean_file_name(item_id.replace(":", ".")) + clean_name += ext + else: + clean_name = clean_file_name(name) + if clean_name != name: + logger.warning( + "[cacheFilePath]" + f" changed name {repr(name)} to {repr(clean_name)}") # ^ replace ":" to avoid converting that one to default "_" # ^ will raise error if path instead of name path = os.path.join(cdi_cache_dir, clean_name) @@ -451,7 +470,7 @@ def cache_cdi_path(cls, item_id: Union[NodeID, str], my_cache_dir=None): # just to be safe, even though clean_file_name # should prevent. If this occurs, fix clean_file_name. raise ValueError("Cannot specify absolute path.") - return path + ".xml" + return path def startElement(self, name: str, attrs: xml.sax.xmlreader.AttributesImpl): @@ -471,7 +490,7 @@ def startElement(self, name: str, if offset is not None: parts.append(f"offset={offset}") logger.debug(*parts) - if attrs is not None and attrs : + if (attrs is not None) and attrs.getNames(): logger.debug(tab, " Attributes: ", attrs.getNames()) # el = ET.Element(name, attrs) @@ -502,6 +521,7 @@ def startElement(self, name: str, self.onPushScope(cm) if len(self._tag_stack) < 1: + assert self._root_memos is not None, "onStart must run first" self._root_memos.append(cm) if cm.tag == "cdi": self._root_memo = cm @@ -623,3 +643,109 @@ def characters(self, content: str): raise TypeError( "Expected str, got {}".format(type(content).__name__)) self._chunks.append(content) + + def expandedTree(self) -> ET.Element: + """Build an expanded XML tree with replication and addresses. + + Starting from the root CDIMemo (via :meth:`getRootMemo`), this + method creates a new ElementTree. Replication is expanded, + addresses are calculated per the OpenLCB CDI standard, and + ``address`` attributes are added where required. + + The ``replication`` attribute is removed from all copied group + elements in the new tree. The original tree is left unchanged. + + Returns: + ET.Element: Root of the new expanded tree. + """ + root_memo = self.getRootMemo() + assert root_memo is not None and root_memo.element is not None + + new_root = ET.Element(root_memo.element.tag) + new_root.attrib.update(root_memo.element.attrib) + + self.address = 0 + + self._expanded_tree_recursive(root_memo, new_root) + return new_root + + def _expanded_tree_recursive( + self, memo: CDIMemo, new_parent: ET.Element + ) -> None: + """Recursive helper for :meth:`expandedTree`. + + Copies the element, handles replication, sets addresses, and + recurses into children. Removes ``replication`` attribute from + copied group elements. + """ + assert memo.element is not None + tag = memo.getTag() or memo.element.tag + tag_lower = tag.lower() + + replication_str = memo.element.attrib.get("replication") + count = int(replication_str) if replication_str is not None else 1 + + if tag_lower == "group": + origin = memo.element.attrib.get("origin") + self.address = int(origin) if origin is not None else 0 + + for idx in range(count): + elem_copy = ET.Element(tag) + elem_copy.attrib.update(memo.element.attrib) + + # Remove replication from the expanded copy + if "replication" in elem_copy.attrib: + del elem_copy.attrib["replication"] + + if tag_lower == "group" or memo.tag in CLASSNAME_TYPES: + elem_copy.set("address", str(self.address)) + + if replication_str is not None and tag_lower == "group": + elem_copy.set("replication_index", str(idx)) + + new_parent.append(elem_copy) + + # Determine size for address advancement + if tag_lower == "eventid": + size = 8 + elif "size" in memo.element.attrib: + size = int(memo.element.attrib["size"]) + else: + size = 0 + + # Recurse into children (replication handled at this level) + for child_memo in memo.children: + self._expanded_tree_recursive(child_memo, elem_copy) + + # Advance address for leaf variables + if tag_lower in CLASSNAME_TYPES or tag_lower == "eventid": + self.address += size + + def extractCDIVarMemos(self) -> List[CDIMemo]: + """Build a flat list of CDIMemo objects for all variables. + + Uses :meth:`expandedTree` internally so replication is fully + expanded and stores it as ``self.expanded_root``. + + Returns: + List of CDIMemo objects (with elements from the expanded + tree). + """ + if not hasattr(self, "etree") or self.etree is None: + logger.error("processor has no etree") + return [] + + self.expanded_root = self.expandedTree() + + cdivar_memos: List[CDIMemo] = [] + + def traverse(element: ET.Element) -> None: + tag_lower = element.tag.lower() + if tag_lower in CLASSNAME_TYPES: + memo = CDIMemo(tag=element.tag, element=element) + cdivar_memos.append(memo) + for child in element: + traverse(child) + + traverse(self.expanded_root) + return cdivar_memos From 1682c765c40774fbdae343ac371b13cebd6e2dbe Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 24 Apr 2026 20:04:05 -0400 Subject: [PATCH 11/34] Make a DataProcessorMemo superclass for parsing messages that are not XML nodes. --- openlcb/dataprocessormemo.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 openlcb/dataprocessormemo.py diff --git a/openlcb/dataprocessormemo.py b/openlcb/dataprocessormemo.py new file mode 100644 index 0000000..32b2869 --- /dev/null +++ b/openlcb/dataprocessormemo.py @@ -0,0 +1,32 @@ +from typing import Union + +from openlcb.message import Message + + +class DataProcessorMemo: + """Store parsing state info. + This superclass can be used for progress notification. + + Attributes: + done (bool): If True, download such as downloadCDI is finished. + Though document itself may be incomplete if 'error' is also + set, stop tracking status of download regardless. + end (bool): False to start a deeper scope, or True for end tag, + which exits current scope (last created Treeview branch in + this case, or top if getBranch() would be None). + error (str): Message of failure (requires 'done' if stopped). + message (Message): Associated network/internal message. + name (str): Name (determined by `name` child element content). + status (str): Status message. + """ + def __init__(self, status: Union[str, None] = None): + self.done = False # type: bool + self.end = False # type: bool + self.error = None # type: str|None + self.message: Union[Message, None] = None # type: Message|None + self.status = status # type: str|None + # region set by DataProcessor such as XMLDataProcessor + self.progress_ratio = None # type: float|None + self.progress_count = None # type: int|None + self.expected_size = None # type: int|None + # end region set by DataProcessor such as XMLDataProcessor From f54ee8c7260869549b013923825956738decad6f Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 24 Apr 2026 20:04:34 -0400 Subject: [PATCH 12/34] Improve emit_cast. --- openlcb/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openlcb/__init__.py b/openlcb/__init__.py index 788de04..309c515 100644 --- a/openlcb/__init__.py +++ b/openlcb/__init__.py @@ -36,6 +36,8 @@ def only_hex_pairs(value: str) -> Union[re.Match[bytes], re.Match[str], None]: def emit_cast(value) -> str: """Get type and value, such as for debug output.""" + if value is None: + return "None" repr_str = repr(value) if isinstance(value, Enum): repr_str = "{}".format(value.value) From 3ac3776516277f5872fa5c65ac7ea7579b943d66 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 24 Apr 2026 20:32:20 -0400 Subject: [PATCH 13/34] Fix replication recursion. Fix: Only set CDIMemo address during replication. --- examples/example_cdi_access.py | 6 +- openlcb/cdimemo.py | 101 ++++++++++--- openlcb/cdivar.py | 1 + openlcb/openlcbnetwork.py | 36 +++-- openlcb/xmldataprocessor.py | 249 ++++++++++++++++++++++----------- 5 files changed, 279 insertions(+), 114 deletions(-) diff --git a/examples/example_cdi_access.py b/examples/example_cdi_access.py index a931f0a..4bbf5f2 100644 --- a/examples/example_cdi_access.py +++ b/examples/example_cdi_access.py @@ -190,10 +190,8 @@ class MyHandler(xml.sax.handler.ContentHandler): _chunks (list[str]): Collects chunks of data. This is implementation-specific, and not required if streaming (parser.feed). - _tmp_address (int|None): Where we are in the memory space (starting - at origin, and calculated using offset and/or size of start - tags). - _tmp_space (int|None): What space we are currently on. + _tmp_address (int|None): For sanity check, not actual address. + See expandedTree docstring. """ def __init__(self): diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index 8112400..9803d1f 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -5,10 +5,14 @@ import xml.etree.ElementTree # import xml.etree.ElementTree as ET -from typing import List, Optional, Union +from typing import Dict, List, Optional, Union +from logging import getLogger -from openlcb.cdivar import FLOAT_MAXIMUMS, NUM_TYPES, CDIVar +from openlcb.cdivar import CLASSNAME_TYPES, FLOAT_MAXIMUMS, NUM_TYPES, CDIVar from openlcb.message import Message +from openlcb.dataprocessormemo import DataProcessorMemo + +logger = getLogger(__name__) def element_ordered(el: xml.etree.ElementTree.Element): @@ -18,7 +22,7 @@ def element_ordered(el: xml.etree.ElementTree.Element): return od -class CDIMemo: +class CDIMemo(DataProcessorMemo): """Store parsing state info as a tree (This is a tree node) Attributes: @@ -41,32 +45,34 @@ class CDIMemo: error (str): Message of failure (requires 'done' if stopped). iid (str): Treeview branch id (no parent when top of Treeview) name (str): Name (determined by `name` child element content). + space (int|None): The memory space address (May be one in + MemorySpace values, or not if vendor-specific such as + defined in CDI etc. See expandedTree in XMLDataProcessor). stray (bool): The end tag is misplaced (doesn't match a start tag) due to bad xml or incorrect parsing. + tail (str|None): Content following the end tag (not used in + OpenLCB CDI/FDI standards). """ def __init__(self, tag: Union[str, None] = None, element: Union[xml.etree.ElementTree.Element, None] = None, status: Union[str, None] = None, - parent: Optional['CDIMemo'] = None): + parent: Optional['CDIMemo'] = None, + document: Optional['XMLDocumentProcessor'] = None): + DataProcessorMemo.__init__(self) self.tag = tag # type: str|None # self.name = None # type: str|None self.element = element # type: xml.etree.ElementTree.Element|None - self.status = status # type: str|None - self.error = None # type: str|None - self.done = False # type: bool - self.end = False # type: bool self.parent = parent # type: CDIMemo|None self.stray = False # type: bool self.content = None # type: str|None - self.message: Union[Message, None] = None # type: Message|None + self.tail = None # type: str|None + # TODO: Set tail (unused in OpenLCB CDI/FDI standards, but allowed in XML) self.iid = None # type: str|None self.address = None # type: int|None + self.space = None # type: int|None self.cdivar = None # type: CDIVar|None self.children = [] # type: List[CDIMemo] - # Set by DataProcessor such as XMLDataProcessor: - self.progress_ratio = None # type: float|None - self.progress_count = None # type: int|None - self.expected_size = None # type: int|None + self.document: Union[Optional['XMLDocumentProcessor'], None] = document def getTag(self): if self.element is None: @@ -101,12 +107,30 @@ def getChildContent(self, tag) -> Union[str, None]: return None def copy(self): + return self.__copy__() + + def __copy__(self): cm = CDIMemo() for k, v in self.__dict__.items(): - if isinstance(v, (list, dict, OrderedDict)): - setattr(cm, k, copy.deepcopy(v)) - else: - setattr(cm, k, v) + setattr(cm, k, v) + return cm + + def __deepcopy__(self, memo: dict): + """Allow deepcopy on this class. + Place id of new object in memo dict + (prevents infinite recursion). + See . + """ + cm = type(self)() + memo[id(self)] = cm + for k, v in self.__dict__.items(): + if k == 'parent': + # prevent invalid container + continue + if k == "document": + # prevent un-pickle-able object (& invalid container) + continue + setattr(cm, k, copy.deepcopy(v, memo)) return cm def getBranch(self, default=None) -> Union[str, None]: @@ -147,6 +171,8 @@ def to_dict(cm): # continue if k == 'parent': continue + if k == 'document': + continue if isinstance(v, xml.etree.ElementTree.Element): d[k] = element_ordered(v) continue @@ -157,8 +183,12 @@ def __str__(self): return json.dumps(CDIMemo.to_dict(self), default=CDIMemo.to_dict) def toCDIVar(self): + # type: () -> CDIVar """Create a CDIVar from descriptors (child elements of self). See LCC "Configuration Description Information" Standard. + + NOTE: The `address` is only correct if this CDIMemo has been + replicated (such as in expandedTree or self.expanded_root). """ # result = CDIVar(self.tag) assert (self.tag is not None) and (self.tag.strip()) @@ -189,8 +219,12 @@ def toCDIVar(self): # enforces size: result = CDIVar(self.tag, _min=result_min, _max=result_max, _size=result_size, _default=result_default) + result.address = self.address # only set in expandedTree() + result.space = self.space result.floatFormat = result_floatFormat result.name = self.getChildContent("name") + if not result.name and (self.tag in CLASSNAME_TYPES): + raise NotImplementedError(f"Can't get name for {self}") if result.className == "int": if result.min is None: @@ -230,3 +264,36 @@ def getSize(self): if size is None: return None return int(size) + + def addChildren(self) -> None: + """Recursively build the full CDIMemo tree from self.element. + + Populates ``self.children`` with proper CDIMemo instances + (one per direct child element). Each child memo also gets + its own children built recursively. + + Preserves original ``.content`` from the parsed tree. + """ + if self.element is None: + self.children = [] + return + + self.children = [] + if self.element.text: + self.content = self.element.text + elif self.element.tag.lower() in ("name", "description"): + logger.warning( + f"{self.element.tag} has no content.") + + if self.element.tail: + self.tail = self.element.tail + + for child_elem in list(self.element): # list() to avoid modification issues + child_memo = CDIMemo( + tag=child_elem.tag, + element=child_elem, + parent=self, + document=self.document + ) + child_memo.addChildren() # recursive + self.children.append(child_memo) diff --git a/openlcb/cdivar.py b/openlcb/cdivar.py index 2c667b0..3a51db9 100644 --- a/openlcb/cdivar.py +++ b/openlcb/cdivar.py @@ -99,6 +99,7 @@ def __init__(self, className, _min=None, _max=None, self.floatFormat = None # type: str|None self.address = None # type: int|None self.element = None # type: Any|None + self.space = None # type: int|None def setData(self, data: Union[bytes, bytearray]): assert isinstance(data, (bytes, bytearray)) diff --git a/openlcb/openlcbnetwork.py b/openlcb/openlcbnetwork.py index fd70f72..33f2806 100644 --- a/openlcb/openlcbnetwork.py +++ b/openlcb/openlcbnetwork.py @@ -28,6 +28,7 @@ from openlcb.cdimemo import CDIMemo from openlcb.datagramservice import DatagramReadMemo, DatagramService from openlcb.dataprocessor import DataFormat +from openlcb.dataprocessormemo import DataProcessorMemo from openlcb.memoryservice import MemoryReadMemo, MemoryService, MemorySpace from openlcb.message import Message from openlcb.xmldataprocessor import XMLDataProcessor @@ -54,7 +55,7 @@ class OpenLCBNetwork: is a MemorySpace) """ def __init__(self, localNodeID: Union[str, bytearray, int, NodeID]): - self._onConnect: Union[Callable[[CDIMemo], None], None] = None + self._onConnect: Union[Callable[[DataProcessorMemo], None], None] = None self._port: PortInterface = None self.physicalLayer: CanPhysicalLayerGridConnect = None self.canLink: CanLink = None @@ -289,23 +290,28 @@ def _listen(self): # manually. # - Usually "socket connection broken" due to no more # bytes to read, but ok if "\0" terminator was reached. - if ((self._dataProcessor._data is not None) - and (not self._dataProcessor._stringTerminated)): - # This boolean is managed by the memoryReadSuccess - # callback. - cm = CDIMemo() - cm.error = formatted_ex(ex) - cm.done = True # stop progress in gui/other main thread - if self._dataProcessor.onStatusMemo: - self._dataProcessor.onStatusMemo(cm) - raise # re-raise since incomplete (prevent done OK state) + if self._dataProcessor is not None: + if ((self._dataProcessor._data is not None) + and (not self._dataProcessor._stringTerminated)): + # This boolean is managed by the memoryReadSuccess + # callback. + cm = DataProcessorMemo() + cm.error = formatted_ex(ex) + cm.done = True # stop progress in gui/other main thread + if self._dataProcessor.onStatusMemo: + self._dataProcessor.onStatusMemo(cm) + raise # re-raise since incomplete (prevent done OK state) + else: + logger.warning( + "Listen loop ended, but _dataProcessor not set" + " (DataProcessorMemo will not be used to notify caller).") finally: self.physicalLayer.physicalLayerDown() # Link_Layer_Down, setState self._listenThread: Union[threading.Thread, None] = None # If we got here, the RuntimeError was ok since the # null terminator '\0' was reached (otherwise re-raise occurs above) - cm = CDIMemo() + cm = DataProcessorMemo() cm.error = ("Listen loop stopped (caught_ex={})." .format(formatted_ex(caught_ex))) cm.done = True @@ -336,7 +342,7 @@ def _handleMessage(self, message: Message): logger.debug("[_handleMessage] message.mti={}".format(message.mti)) if message.mti == MTI.Link_Layer_Down: if self._onConnect: - cm = CDIMemo() + cm = DataProcessorMemo() cm.done = True cm.error = "Disconnected" cm.message = message @@ -345,7 +351,7 @@ def _handleMessage(self, message: Message): return True elif message.mti == MTI.Link_Layer_Up: if self._onConnect: - cm = CDIMemo() + cm = DataProcessorMemo() cm.done = True # 'done' without error indicates connected. cm.message = message self._onConnect(cm) @@ -406,7 +412,7 @@ def _memoryReadFail(self, memo: MemoryReadMemo): if len(self._dataProcessor._tag_stack): cm = self._dataProcessor._tag_stack[-1] else: - cm = CDIMemo() + cm = DataProcessorMemo() cm.error = error cm.done = True # stop progress in gui/other main thread self._dataProcessor._onElement(cm) diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 3054efc..f8bf8a8 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -1,17 +1,18 @@ from collections import OrderedDict +import copy import os import xml.sax # noqa: E402 import xml.sax.handler import xml.etree.ElementTree as ET from logging import getLogger -from typing import Callable, List, Union +from typing import Callable, List, Tuple, Union # from xml.sax.xmlreader import AttributesImpl # for type hints, for autocomplete only in this case # noqa:E501 import xml.sax.xmlreader # for type hints, for autocomplete only in this case from openlcb import d_quote, emit_cast from openlcb.canbus.canlink import CanLink -from openlcb.cdimemo import CDIMemo +from openlcb.cdimemo import CDIMemo, DataProcessorMemo from openlcb.dataprocessor import DataFormat, DataProcessor from openlcb.nodeid import NodeID from openlcb.platformextras import ( @@ -101,10 +102,9 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): _space (int): Space containing the CDI itself (not data described by CDI). _tmp_space (int|None): What space we are currently on - (of data described by Element(s), not of XML data itself) - _tmp_address (int|None): Where we are in the memory space - (starting at origin, and calculated using offset and/or size - of start tags). + (of data described by Element(s), not of XML data itself). + _tmp_address (int|None): For sanity check, not actual address + (no replication)! See expandedTree docstring. """ XML_TOP_TAGS = ("cdi", "fdi") DEFAULT_CACHES_DIR = SysDirs.Cache @@ -113,7 +113,8 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): def __init__(self, linkLayer: CanLink, space: MemorySpace): self.canLink: CanLink = linkLayer # caches_dir = SysDirs.Cache - self.expanded_root = None # type: xml.etree.ElementTree|None + self.expanded_root = None # type: ET.Element|None + self.expanded_root_memo = None # type: CDIMemo|None self._root_memos = None # type: list[CDIMemo]|None self._root_memo = None # type: CDIMemo|None self._space: Union[MemorySpace, None] = None @@ -202,7 +203,7 @@ def space(self) -> MemorySpace: assert isinstance(self._space, MemorySpace) return self._space - def onStatusMemo(self, cm: CDIMemo) -> bool: + def onStatusMemo(self, cm: DataProcessorMemo) -> bool: """Handle memo with status that doesn't affect tag stack/scope. (Implement in subclass) Returns: @@ -297,7 +298,7 @@ def _feedNext(self, memo: MemoryReadMemo): partial_str = memo.data.decode("utf-8") if self._realtime: self._parser.feed(partial_str) # may call startElement/endElement - cm = CDIMemo() + cm = DataProcessorMemo() cm.progress_count = self.progress_count cm.expected_size = self.expected_size self.onStatusMemo(cm) @@ -395,7 +396,7 @@ def _feedLast(self, memo: MemoryReadMemo, enable_cache=None): partial_str = memo.data[:terminate_i].decode("utf-8") assert self.progress_count is not None self.progress_count += terminate_i - cm = CDIMemo() + cm = DataProcessorMemo() cm.done = True # 'done' and not 'error' means got all cm.progress_count = self.progress_count cm.expected_size = self.expected_size @@ -419,7 +420,7 @@ def _feedLast(self, memo: MemoryReadMemo, enable_cache=None): # ?xml version="1.0" encoding="utf-8"?> xml.sax.parseString(cdiString, self) # self._fireStatus("Done loading CDI.") - cm = CDIMemo() + cm = DataProcessorMemo() cm.done = True # 'done' and not 'error' means got all cm.progress_count = self.progress_count self.onStatusMemo(cm) @@ -501,7 +502,7 @@ def startElement(self, name: str, parent_cm = None if self._tag_stack: parent_cm = self._tag_stack[-1] - cm = CDIMemo(tag=name, element=el, parent=parent_cm) + cm = CDIMemo(tag=name, element=el, parent=parent_cm, document=self) if name == "segment": self._tmp_space = attrib.get('space') self._tmp_address = int(attrib.get('origin', 0)) @@ -517,7 +518,7 @@ def startElement(self, name: str, raise AttributeError( f"Node specifies {name} offset before segment origin") self._tmp_address += offset - cm.address = self._tmp_address # May be None if after /segment + # NOTE: ^ Sanity check only! For real address see expandedTree. self.onPushScope(cm) if len(self._tag_stack) < 1: @@ -583,7 +584,7 @@ def endElement(self, name: str): top_cm.tag = name cm = top_cm else: - cm = CDIMemo(tag=name) + cm = CDIMemo(tag=name, document=self) cm.stray = True cm.end = True @@ -644,7 +645,7 @@ def characters(self, content: str): "Expected str, got {}".format(type(content).__name__)) self._chunks.append(content) - def expandedTree(self) -> ET.Element: + def expandedTree(self) -> Tuple[CDIMemo, ET.Element]: """Build an expanded XML tree with replication and addresses. Starting from the root CDIMemo (via :meth:`getRootMemo`), this @@ -661,91 +662,183 @@ def expandedTree(self) -> ET.Element: root_memo = self.getRootMemo() assert root_memo is not None and root_memo.element is not None - new_root = ET.Element(root_memo.element.tag) + new_root = ET.Element("cdi") # always new: children added from memos new_root.attrib.update(root_memo.element.attrib) - self.address = 0 - - self._expanded_tree_recursive(root_memo, new_root) - return new_root + new_root_memo = copy.deepcopy(root_memo) # deepcopy to edit children! + new_root_memo.document = self + size = self._expanded_tree_recursive(new_root_memo, new_root, address=0) + if size < 1: + logger.warning(f"No space used by CDI after replication (size={size})") + return new_root_memo, new_root def _expanded_tree_recursive( - self, memo: CDIMemo, new_parent: ET.Element - ) -> None: + self, parent: CDIMemo, parent_el: ET.Element, + allow_non_standard=False, + address: int = 0, + space: Union[int, None] = None, + ) -> int: """Recursive helper for :meth:`expandedTree`. Copies the element, handles replication, sets addresses, and recurses into children. Removes ``replication`` attribute from copied group elements. """ - assert memo.element is not None - tag = memo.getTag() or memo.element.tag - tag_lower = tag.lower() - - replication_str = memo.element.attrib.get("replication") - count = int(replication_str) if replication_str is not None else 1 - - if tag_lower == "group": - origin = memo.element.attrib.get("origin") - self.address = int(origin) if origin is not None else 0 - - for idx in range(count): - elem_copy = ET.Element(tag) - elem_copy.attrib.update(memo.element.attrib) - - # Remove replication from the expanded copy - if "replication" in elem_copy.attrib: - del elem_copy.attrib["replication"] - - if tag_lower == "group" or memo.tag in CLASSNAME_TYPES: - elem_copy.set("address", str(self.address)) - - if replication_str is not None and tag_lower == "group": - elem_copy.set("replication_index", str(idx)) - - new_parent.append(elem_copy) - - # Determine size for address advancement - if tag_lower == "eventid": - size = 8 - elif "size" in memo.element.attrib: - size = int(memo.element.attrib["size"]) - else: - size = 0 - - # Recurse into children (replication handled at this level) - for child_memo in memo.children: - self._expanded_tree_recursive(child_memo, elem_copy) - - # Advance address for leaf variables - if tag_lower in CLASSNAME_TYPES or tag_lower == "eventid": - self.address += size - - def extractCDIVarMemos(self) -> List[CDIMemo]: + assert address is not None + assert parent.element is not None + parent_tag = parent.getTag() or parent.element.tag + parent_tag_lower = parent_tag.lower() + if parent_el.text: + parent.content = parent_el.text + elif parent.content: # new_root + parent_el.text = parent.content + elif parent_tag_lower in ("name", "description"): + logger.warning( + f"expanded {parent_tag_lower} has no content.") + if parent_el.tail: + parent.tail = parent_el.tail + elif parent.tail: # # new_root + parent_el.tail = parent.tail + + # Recurse into children (replication handled at this level) + new_children = [] + # new_child_elements = [] + for child_memo in parent.children: + replication_str = parent.element.attrib.get("replication") + count = int(replication_str) if replication_str is not None else 1 + child_tag = child_memo.getTag() + assert child_tag + c_tag_lower = child_tag.lower() + child_el = child_memo.element + assert child_el is not None + if c_tag_lower == "segment": + space_str = child_el.attrib.get("space") + assert space_str, "expected space in segment" + space = int(space_str) + address = 0 # as per standard, 1st is at 0 else use "group" + if c_tag_lower == "group": + origin = child_el.attrib.get("origin") + address = int(origin) if (origin is not None) else 0 + for idx in range(count): + # if count > 1: + copy_child_el = ET.Element(child_el.tag) + copy_child_el.attrib.update(child_el.attrib) + copy_child_el.text = child_el.text + copy_child_el.tail = child_el.tail + copy_child_memo = copy.deepcopy(child_memo) + copy_child_memo.parent = parent + copy_child_memo.document = self + copy_child_memo.element = copy_child_el + # else: + # copy_child_el = child_el + # copy_child_memo = child_memo + # NOTE: ^ Why commented: We don't want to modify + # self.etree children (if we modify expandedTree + # result such as self.expanded_root)! + # - Don't even chance it by keeping the memo + # (otherwise child_memo.element would be from tree). + # - Also, we always add child to parent_el below. + + new_children.append(copy_child_memo) + # for child_el in new_parent: + # copy_parent_el.append(child_el) + + # Remove replication from the expanded copy + if "replication" in copy_child_el.attrib: + del copy_child_el.attrib["replication"] + + if c_tag_lower == "group" or c_tag_lower in CLASSNAME_TYPES: + copy_child_el.set("address", str(address)) + copy_child_el.set("space", str(space)) + if c_tag_lower in CLASSNAME_TYPES: + copy_child_memo.address = address + copy_child_memo.space = space + + if replication_str is not None: + if c_tag_lower != "group": + el_error = \ + f"unexpected replication for {c_tag_lower} tag" + if allow_non_standard: + logger.warning(el_error) + else: + raise SyntaxError(el_error) + copy_child_el.set('replication_index', str(idx)) + # ^ optimized attrib[key] = value + + parent_el.append(copy_child_el) + # parent: Use new_children below (can't change while iterating) + + # Determine size for address advancement + if c_tag_lower == "eventid": + size = 8 + elif "size" in copy_child_el.attrib: + size = int(copy_child_el.attrib["size"]) + else: + size = 0 + + # Advance address *before* leaf variables + if size: + if c_tag_lower in CLASSNAME_TYPES: + assert size, f"expected size for {c_tag_lower}" + address += size + else: + el_error = ( + f"size is not expected for {c_tag_lower}" + f" size={size}") + if allow_non_standard: + logger.warning(el_error) + address += size + else: + assert not size, el_error + + address = self._expanded_tree_recursive( + copy_child_memo, copy_child_el, address=address, + space=space) + if c_tag_lower == "segment": + space = None # undefined after section + + parent.children = new_children # Same references if no replication + return address + + def extractCDIVarMemos(self, expanded_root=None, root_memo=None) -> List[CDIMemo]: # noqa: E501 + # type: (ET.Element|None, CDIMemo|None) -> List[CDIMemo] """Build a flat list of CDIMemo objects for all variables. - Uses :meth:`expandedTree` internally so replication is fully - expanded and stores it as ``self.expanded_root``. - - Returns: - List of CDIMemo objects (with elements from the expanded - tree). + Uses the expanded tree (replication expanded, replication + attribute removed, addresses set). Returns original-style + memos (with .content) but with .element pointing into the + expanded tree so that modifications (setData etc.) affect + the saved XML. """ + # TODO: Implement ACDI vars if present (See OpenLCB + # "Configuration Description Information" Standard) if not hasattr(self, "etree") or self.etree is None: logger.error("processor has no etree") return [] + if expanded_root is not None: + if root_memo is not None: # reserved + assert isinstance(root_memo, CDIMemo) + assert isinstance(expanded_root, ET.Element) + root_memo = root_memo # reserved + root_el = expanded_root + else: + root_memo, root_el = self.expandedTree() + self.expanded_root = root_el + self.expanded_root_memo = root_memo - self.expanded_root = self.expandedTree() + assert isinstance(self.expanded_root_memo, CDIMemo) cdivar_memos: List[CDIMemo] = [] - def traverse(element: ET.Element) -> None: - tag_lower = element.tag.lower() + def traverse(memo: CDIMemo) -> None: + tag = memo.getTag() + tag_lower = tag.lower() if tag else "" if tag_lower in CLASSNAME_TYPES: - memo = CDIMemo(tag=element.tag, element=element) + # Use the existing expanded memo (has correct .content) cdivar_memos.append(memo) - for child in element: + for child in memo.children: traverse(child) - traverse(self.expanded_root) + traverse(self.expanded_root_memo) + assert root_el is self.expanded_root # concurrent modification check return cdivar_memos From a7d0cd664ec827a7b339b3084f42623fb95d66f1 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Mon, 27 Apr 2026 14:13:45 -0400 Subject: [PATCH 14/34] Fix: Correctly separate tail of XML--prevents formatting from modifying values (text content). --- openlcb/xmldataprocessor.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index f8bf8a8..bc003a6 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -88,6 +88,12 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): _openEl (SubElement): Tracks currently-open tag (no `` yet) during parsing, or if no tags are open then equals etree. + _ended_memo (CDIMemo|None): The memo most recently popped, + where "tail" (text after end tag) should be set during + "characters" when a new tag hasn't been started yet. + TODO: Put child element's tail in parent (Not part of + Standard as of 2026-05, but technically possible, such + as "World" in `Hello
World`). _tag_stack (list[SubElement]): Tracks scope during parse since self.etree doesn't have awareness of whether end tag is finished (and therefore doesn't know which element is the @@ -121,6 +127,7 @@ def __init__(self, linkLayer: CanLink, space: MemorySpace): self._openEl: Union[ET.Element, None] = None self._top_tag = "cdi" # cdi or fdi (detected in startElement) # self._myCacheDir = os.path.join(caches_dir, "python-openlcb") + self._ended_memo = None # type: CDIMemo|None self._myCacheDir = XMLDataProcessor.DEFAULT_CACHE_DIR self._tmp_space = None # type: int|None self._tmp_address = None # type: int|None @@ -482,6 +489,15 @@ def startElement(self, name: str, self._top_tag = name.lower() elif name.lower() == "acdi": self.acdi = True + tail = self._flushCharBuffer() if self._chunks else None + if tail is not None: + if self._ended_memo is not None: + self._ended_memo.tail = tail + else: + logger.warning( + f"Stray characters before {repr(name)}: {repr(tail)}") + if self._ended_memo is not None: + self._ended_memo = None attrib = attrs_to_dict(attrs) origin = attrib.get('origin') offset = attrib.get('offset') @@ -618,6 +634,7 @@ def endElement(self, name: str): cm.parent.children.append(cm) _ = self.checkDone(cm) cm.content = self._flushCharBuffer() + self._ended_memo = cm self.onPopScope(cm) def _flushCharBuffer(self): @@ -697,7 +714,7 @@ def _expanded_tree_recursive( f"expanded {parent_tag_lower} has no content.") if parent_el.tail: parent.tail = parent_el.tail - elif parent.tail: # # new_root + elif parent.tail: # new_root parent_el.tail = parent.tail # Recurse into children (replication handled at this level) @@ -724,7 +741,11 @@ def _expanded_tree_recursive( copy_child_el = ET.Element(child_el.tag) copy_child_el.attrib.update(child_el.attrib) copy_child_el.text = child_el.text + if child_el.text is not None: + copy_child_el.text = child_el.text.strip() copy_child_el.tail = child_el.tail + if child_el.tail is not None: + copy_child_el.tail = child_el.tail.strip() copy_child_memo = copy.deepcopy(child_memo) copy_child_memo.parent = parent copy_child_memo.document = self From d705a20e91aae34865915cd6371903acf8ffc7d1 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Mon, 27 Apr 2026 16:11:30 -0400 Subject: [PATCH 15/34] Fix: Correctly handle origin and offset. Fix: Correctly preserve XML formatting. --- openlcb/xmldataprocessor.py | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index bc003a6..82a3cb8 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -489,13 +489,21 @@ def startElement(self, name: str, self._top_tag = name.lower() elif name.lower() == "acdi": self.acdi = True - tail = self._flushCharBuffer() if self._chunks else None - if tail is not None: + content = self._flushCharBuffer() if self._chunks else None + if content is not None: if self._ended_memo is not None: - self._ended_memo.tail = tail + self._ended_memo.tail = content else: - logger.warning( - f"Stray characters before {repr(name)}: {repr(tail)}") + if self._tag_stack: + # Text in parent before this + # (typically "\n", possibly indentation). + if self._tag_stack[-1].content is None: + self._tag_stack[-1].content = content + else: + self._tag_stack[-1].content += content + else: + logger.warning( + f"Stray characters before {repr(name)}: {repr(content)}") if self._ended_memo is not None: self._ended_memo = None attrib = attrs_to_dict(attrs) @@ -721,7 +729,7 @@ def _expanded_tree_recursive( new_children = [] # new_child_elements = [] for child_memo in parent.children: - replication_str = parent.element.attrib.get("replication") + replication_str = parent.element.attrib.get('replication') count = int(replication_str) if replication_str is not None else 1 child_tag = child_memo.getTag() assert child_tag @@ -729,13 +737,15 @@ def _expanded_tree_recursive( child_el = child_memo.element assert child_el is not None if c_tag_lower == "segment": - space_str = child_el.attrib.get("space") + space_str = child_el.attrib.get('space') assert space_str, "expected space in segment" space = int(space_str) - address = 0 # as per standard, 1st is at 0 else use "group" - if c_tag_lower == "group": - origin = child_el.attrib.get("origin") + origin = child_el.attrib.get('origin') address = int(origin) if (origin is not None) else 0 + if c_tag_lower == "group": + offset = child_el.attrib.get('offset') + if offset: + address += int(offset) for idx in range(count): # if count > 1: copy_child_el = ET.Element(child_el.tag) @@ -769,8 +779,8 @@ def _expanded_tree_recursive( del copy_child_el.attrib["replication"] if c_tag_lower == "group" or c_tag_lower in CLASSNAME_TYPES: - copy_child_el.set("address", str(address)) - copy_child_el.set("space", str(space)) + copy_child_el.set('address', str(address)) + copy_child_el.set('space', str(space)) if c_tag_lower in CLASSNAME_TYPES: copy_child_memo.address = address copy_child_memo.space = space From de593f951daab0daf8d5f2c2bcf2f5bd7f0aa8af Mon Sep 17 00:00:00 2001 From: Bob Jacobsen Date: Thu, 30 Apr 2026 10:30:40 -0400 Subject: [PATCH 16/34] only act on datagram replies matched to our requests --- openlcb/datagramservice.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openlcb/datagramservice.py b/openlcb/datagramservice.py index dd70f4f..8b0ae74 100644 --- a/openlcb/datagramservice.py +++ b/openlcb/datagramservice.py @@ -246,6 +246,9 @@ def handleDatagramReceivedOK(self, message: Message): # match to the memo and remove from queue memo = self.matchToWriteMemo(message) # type: DatagramWriteMemo|None + # check for whether a match was found, indicating this was for us + if memo is None : return + # check of tracking logic if self.currentOutstandingMemo != memo: logger.error( From 45783bd732a27e45a11e146b0a8fddec9e81945d Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:41:10 -0400 Subject: [PATCH 17/34] Improve hr_repr. Separate cacheFileName code. --- openlcb/__init__.py | 10 ++++++---- openlcb/xmldataprocessor.py | 18 ++++++++++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/openlcb/__init__.py b/openlcb/__init__.py index 309c515..325ed0f 100644 --- a/openlcb/__init__.py +++ b/openlcb/__init__.py @@ -139,10 +139,12 @@ def hr_repr(value, always_quote: bool = False) -> str: """Represent value with double quotes (Human-readable repr). """ - repr_value = repr(value) - if repr_value.startswith("'") and repr_value.endswith("'"): - return '"' + repr_value[1:-1].replace('"', '\\"') + '"' - elif always_quote: + # repr_value = repr(value) + # repr_value = repr_value.replace("\\\\", "\\") + # if repr_value.startswith("'") and repr_value.endswith("'"): + # return '"' + repr_value[1:-1].replace('"', '\\"') + '"' + repr_value = str(value) + if always_quote or isinstance(value, str): return '"' + repr_value.replace('"', '\\"') + '"' return repr(value) diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 82a3cb8..d08ed62 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -447,13 +447,21 @@ def cacheFilePathCustom(self, item_id: Union[NodeID, str], **kwargs): kwargs['my_cache_dir'] = self._myCacheDir type(self).cacheFilePath(item_id, **kwargs) + @classmethod + def cacheFileName(cls, item_id: Union[NodeID, str], ext=".cdi.xml"): + item_id = str(item_id) # Convert NodeID or other + clean_name = clean_file_name(item_id.replace(":", ".")) + clean_name += ext + return clean_name + @classmethod def cacheFilePath(cls, item_id: Union[NodeID, str], my_cache_dir=None, - subFolder="cdi", name=None, ext=".xml"): + subfolder: Union[str, None] = "cdi", name=None, + ext=".cdi.xml"): if my_cache_dir is None: my_cache_dir = cls.DEFAULT_CACHE_DIR - if subFolder: - cdi_cache_dir = os.path.join(my_cache_dir, subFolder) + if subfolder: + cdi_cache_dir = os.path.join(my_cache_dir, subfolder) else: cdi_cache_dir = my_cache_dir if not os.path.isdir(cdi_cache_dir): @@ -462,9 +470,7 @@ def cacheFilePath(cls, item_id: Union[NodeID, str], my_cache_dir=None, # name file to avoid cache file from a different # device/version. if not name: - item_id = str(item_id) # Convert NodeID or other - clean_name = clean_file_name(item_id.replace(":", ".")) - clean_name += ext + clean_name = cls.cacheFileName(item_id, ext=ext) else: clean_name = clean_file_name(name) if clean_name != name: From 34a17cc1c07b752cef62283c9edcbac1ae5e6c68 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Thu, 30 Apr 2026 16:47:25 -0400 Subject: [PATCH 18/34] Make default extension specific to DataProcessor subclass. --- openlcb/dataprocessor.py | 2 ++ openlcb/tcplink/mdnsconventions.py | 3 ++- openlcb/xmldataprocessor.py | 9 +++++++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/openlcb/dataprocessor.py b/openlcb/dataprocessor.py index d21e99a..660e350 100644 --- a/openlcb/dataprocessor.py +++ b/openlcb/dataprocessor.py @@ -14,6 +14,8 @@ class DataProcessor: Attributes: enable_cache (bool): Defaults to False (May differ in subclass). """ + DEFAULT_EXT = ".bin" # override in subclass + def __init__(self): self.enable_cache = False # type: bool # Members used to construct space memo such as CDIMemo: diff --git a/openlcb/tcplink/mdnsconventions.py b/openlcb/tcplink/mdnsconventions.py index 6cf4380..ff70caf 100644 --- a/openlcb/tcplink/mdnsconventions.py +++ b/openlcb/tcplink/mdnsconventions.py @@ -1,4 +1,5 @@ from logging import getLogger +from typing import Union from openlcb import only_hex_pairs from openlcb.conventions import hex_to_dotted_lcc_id @@ -7,7 +8,7 @@ logger = getLogger(__name__) -def id_from_tcp_service_name(service_name): +def id_from_tcp_service_name(service_name) -> Union[str, None]: """Scrape an MDNS TCP service name, assuming it uses conventions (`"{org}_{model}_{id}._openlcb-can.{protocol}.{tld}".format(...)` where: diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index d08ed62..bb0be8d 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -113,6 +113,7 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): (no replication)! See expandedTree docstring. """ XML_TOP_TAGS = ("cdi", "fdi") + DEFAULT_EXT = ".cdi.xml" # override in subclass DEFAULT_CACHES_DIR = SysDirs.Cache DEFAULT_CACHE_DIR = os.path.join(DEFAULT_CACHES_DIR, "python-openlcb") @@ -448,7 +449,9 @@ def cacheFilePathCustom(self, item_id: Union[NodeID, str], **kwargs): type(self).cacheFilePath(item_id, **kwargs) @classmethod - def cacheFileName(cls, item_id: Union[NodeID, str], ext=".cdi.xml"): + def cacheFileName(cls, item_id: Union[NodeID, str], ext=None): + if ext is None: + ext = cls.DEFAULT_EXT item_id = str(item_id) # Convert NodeID or other clean_name = clean_file_name(item_id.replace(":", ".")) clean_name += ext @@ -457,7 +460,9 @@ def cacheFileName(cls, item_id: Union[NodeID, str], ext=".cdi.xml"): @classmethod def cacheFilePath(cls, item_id: Union[NodeID, str], my_cache_dir=None, subfolder: Union[str, None] = "cdi", name=None, - ext=".cdi.xml"): + ext=None): + if ext is None: + ext = cls.DEFAULT_EXT if my_cache_dir is None: my_cache_dir = cls.DEFAULT_CACHE_DIR if subfolder: From 2feca63f9d38954523fe6724e675436437294e3e Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 11:17:07 -0400 Subject: [PATCH 19/34] Make missing name issue more clear. --- openlcb/platformextras.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openlcb/platformextras.py b/openlcb/platformextras.py index 8cb6b61..142e881 100644 --- a/openlcb/platformextras.py +++ b/openlcb/platformextras.py @@ -53,6 +53,7 @@ def clean_file_name_char(c: str, placeholder: Union[str, None] = None) -> str: def clean_file_name(name: str, placeholder: Union[str, None] = None) -> str: + assert name is not None assert isinstance(name, str) if (os.path.sep in name) or ("/" in name): # or "/" since Python uses that even on Windows From 7c9a1911edc275727ba765dd80e402d2b76c9386 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 13:06:46 -0400 Subject: [PATCH 20/34] Add import used for type hint. --- openlcb/memoryservice.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index 698bf8d..bc1fbef 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -36,6 +36,7 @@ DatagramService, ) from openlcb.convert import Convert +from openlcb.nodeid import NodeID logger = getLogger(__name__) From e55fa784d1fd39a87448e6f6d66839aaafffeed5 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 13:54:48 -0400 Subject: [PATCH 21/34] Check if rejectedReply should be discarded as well (follow-up to similar okReply commit for issue #88). --- openlcb/datagramservice.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/openlcb/datagramservice.py b/openlcb/datagramservice.py index 8b0ae74..743cc21 100644 --- a/openlcb/datagramservice.py +++ b/openlcb/datagramservice.py @@ -248,7 +248,7 @@ def handleDatagramReceivedOK(self, message: Message): # check for whether a match was found, indicating this was for us if memo is None : return - + # check of tracking logic if self.currentOutstandingMemo != memo: logger.error( @@ -267,6 +267,10 @@ def handleDatagramRejected(self, message: Message): # match to the memo and remove from queue memo = self.matchToWriteMemo(message) + # check for whether a match was found, indicating this was for us + if memo is None: + return + # check of tracking logic if self.currentOutstandingMemo != memo: logger.error( From 87ffcd2276efc3d9f4a6e22fe6c78892f2df756c Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 13:59:54 -0400 Subject: [PATCH 22/34] Implement stream modes for Memory Configuration. --- openlcb/convert.py | 10 +++++++--- openlcb/memoryservice.py | 40 +++++++++++++++++++++++++++++----------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/openlcb/convert.py b/openlcb/convert.py index 9f205eb..6ab21e1 100644 --- a/openlcb/convert.py +++ b/openlcb/convert.py @@ -31,9 +31,13 @@ def spaceDecode(space): - 0x00 to 0xFC represent standard memory spaces directly. Returns: - tuple(bool, byte): (False, 1-3 for in command byte) : - spaces 0xFF - 0xFD - or (True, space number) : spaces 0 - 0xFC + tuple(bool, byte): (is custom space, command | space) + - (False, 1-3 for in command byte) : + spaces 0xFF - 0xFD (Except bits beyond 0x00000011 + differ for each datagram type. See 4.2 Address + Space Selection in OpenLCB Memory Configuration + Standard) + - or (True, space number) : spaces 0 - 0xFC (NOTE: type of space may affect type of output) """ # TODO: Maybe check type of space & raise TypeError if not diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index bc1fbef..840533a 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -25,7 +25,8 @@ from logging import getLogger from typing import ( Callable, - List, # in case list doesn't support `[` in this Python version + List, + Optional, # in case list doesn't support `[` in this Python version Union, # in case `|` doesn't support 'type' in this Python version ) @@ -168,8 +169,8 @@ def __init__(self, service: DatagramService): self.datagramReceivedListener ) - def requestMemoryRead(self, memo): - # type: (MemoryReadMemo) -> None + def requestMemoryRead(self, memo, stream: Bool = False): + # type: (MemoryReadMemo, Optional[bool]) -> None '''Request a read operation start. - If okReply in the memo is triggered, it will be followed by a @@ -180,23 +181,31 @@ def requestMemoryRead(self, memo): Args: memo (MemoryReadMemo): Request to enqueue. ''' + assert isinstance(stream, bool) # preserve the request self.readMemos.append(memo) if len(self.readMemos) == 1: - self.requestMemoryReadNext(memo) + self.requestMemoryReadNext(memo, stream=stream) - def requestMemoryReadNext(self, memo): - # type: (MemoryReadMemo) -> None + def requestMemoryReadNext(self, memo, stream: bool = False): + # type: (MemoryReadMemo, Optional[bool]) -> None """send the read request Args: memo (MemoryReadMemo): Request to send. """ - byte6 = False + assert isinstance(stream, bool) + byte6 = False # if custom space is defined in byte 6 flag = 0 (byte6, flag) = Convert.spaceDecode(memo.space) - spaceFlag = 0x40 if byte6 else (flag | 0x40) + if stream: + # Encoding: 0x60=custom, 0x61=0xFD, 0x62=0xFE, 0x63=0xFF + spaceFlag = 0x60 if byte6 else (flag | 0x60) + else: + # Encoding: 0x40=custom, 0x41=0xFD, 0x42=0xFE, 0x43=0xFF + spaceFlag = 0x40 if byte6 else (flag | 0x40) # | 0b11111100 + # ^ In else case, flag is 1-3, so re-add 0xFC (0b11111100) addr2 = ((memo.address >> 24) & 0xFF) addr3 = ((memo.address >> 16) & 0xFF) addr4 = ((memo.address >> 8) & 0xFF) @@ -206,6 +215,7 @@ def requestMemoryReadNext(self, memo): addr2, addr3, addr4, addr5]) # NOTE: list[int] is ok for bytearray extend (`+` requires cast) if byte6: + assert memo.space <= 0xFF, f"Space {memo.space} out of byte range" data.extend([(memo.space & 0xFF)]) data.extend([memo.size]) logger.debug( @@ -313,19 +323,26 @@ def datagramReceivedListener(self, dmemo: DatagramReadMemo) -> bool: return True - def requestMemoryWrite(self, memo: MemoryWriteMemo): + def requestMemoryWrite(self, memo: MemoryWriteMemo, stream: bool = False): + # type: (MemoryWriteMemo, Optional[bool]) -> None """Request memory write. Args: memo (MemoryWriteMemo): information to send """ + assert isinstance(stream, bool) # preserve the request self.writeMemos.append(memo) # create & send a write datagram - byte6 = False + byte6 = False # if custom space is defined in byte 6 flag = 0 (byte6, flag) = Convert.spaceDecode(memo.space) - spaceFlag = 0x00 if byte6 else (flag | 0x00) + if stream: + # Encoding: 0x20=custom, 0x21=0xFD, 0x22=0xFE, 0x23=0xFF + spaceFlag = 0x20 if byte6 else (flag | 0x20) + else: + # Encoding: 0x00=custom, 0x01=0xFD, 0x02=0xFE, 0x03=0xFF + spaceFlag = 0x00 if byte6 else (flag | 0x00) addr2 = ((memo.address >> 24) & 0xFF) addr3 = ((memo.address >> 16) & 0xFF) addr4 = ((memo.address >> 8) & 0xFF) @@ -335,6 +352,7 @@ def requestMemoryWrite(self, memo: MemoryWriteMemo): addr2, addr3, addr4, addr5 ]) if byte6: + assert memo.space <= 0xFF, f"Space {memo.space} out of byte range" data.extend([(memo.space & 0xFF)]) data.extend(memo.data) dgWriteMemo = DatagramWriteMemo(memo.nodeID, data) From 177237039e7eda7fa5027900cf089dd6105380a7 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 16:17:12 -0400 Subject: [PATCH 23/34] Rename spaceDecode to serializeSpace for clarity, and add opposite method: deserializeMC2ndByte. Decode errorCode and collect error string (if present) from Datagram in rejectedReply. Add getBeforeNull for deserializing strings. --- openlcb/convert.py | 35 ++++++++-- openlcb/memoryservice.py | 140 +++++++++++++++++++++++++++++++++++---- tests/test_convert.py | 8 +-- 3 files changed, 159 insertions(+), 24 deletions(-) diff --git a/openlcb/convert.py b/openlcb/convert.py index 6ab21e1..5875477 100644 --- a/openlcb/convert.py +++ b/openlcb/convert.py @@ -20,19 +20,30 @@ class Convert: @staticmethod - def spaceDecode(space): + def deserializeMC2ndByte(datagramByte1): + """Decode byte[1] (2nd) of Memory Configuration Datagram""" + has_byte6 = False + if datagramByte1 & 0x03 == 0: + has_byte6 = True + return has_byte6, datagramByte1 & 0xFC + # ^ 0xFC = 11111100 + + # formerly spaceDecode, but it serializes a space for datagram byte2 + @staticmethod + def serializeSpace(space): """Convert from a space number to either - False and command byte or True and standard memory space + False and control number or True and standard memory space + for use in a Datagram. Args: - space (int): Encoded memory space identifier, where values: + space (int): Sequential memory space identifier, where values: - 0xFF to 0xFD are special spaces, and only the least significant - 2 bits are relevant. + 2 bits will be used in a datagram. - 0x00 to 0xFC represent standard memory spaces directly. Returns: - tuple(bool, byte): (is custom space, command | space) - - (False, 1-3 for in command byte) : + tuple(bool, byte): (is custom space, control | space) + - (False, control number 1 to 3 inclusive) : spaces 0xFF - 0xFD (Except bits beyond 0x00000011 differ for each datagram type. See 4.2 Address Space Selection in OpenLCB Memory Configuration @@ -168,3 +179,15 @@ def stringToArray(value, length): # to bytearray after getting list[int] of remaining length: padding = bytearray([0] * (length-len(contentPart))) return contentPart + padding + + @staticmethod + def getBeforeNull(data: Union[bytes, bytearray], start): + null_idx = -1 + for i in range(start, len(data)): + assert isinstance(data[i], int) + if data[i] == 0: + null_idx = i + break + if null_idx > -1: + return data[start:null_idx] + return data[start:] diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index 840533a..2b98224 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -30,6 +30,9 @@ Union, # in case `|` doesn't support 'type' in this Python version ) +from openlcb import ( + emit_cast, +) from openlcb.datagramservice import ( # DatagramReadMemo, DatagramReadMemo, @@ -42,6 +45,26 @@ logger = getLogger(__name__) +MODE_BYTES = { + 'Read_Command': {0x40, 0x41, 0x42, 0x43}, + 'Read_Reply': {0x50, 0x51, 0x52, 0x53}, + 'Read_Stream_Command': {0x60, 0x61, 0x62, 0x63}, + 'Read_Stream_Reply': {0x70, 0x71, 0x72, 0x73}, + 'Write_Command': {0x00, 0x01, 0x02, 0x03}, + 'Write_Reply': {0x10, 0x11, 0x12, 0x13}, + 'Write_Under_Mask_Command': {0x08, 0x09, 0x0A, 0x0B}, + 'Write_Stream_Command': {0x20, 0x21, 0x22, 0x23}, + 'Write_Stream_Reply': {0x30, 0x31, 0x32, 0x33}, +} + +MODE_ERROR_BYTES = { + 'Read_Reply': {0x58, 0x59, 0x5A, 0x5B}, + 'Read_Stream_Reply': {0x78, 0x79, 0x7A, 0x7B}, + 'Write_Reply': {0x18, 0x19, 0x1A, 0x1B}, + 'Write_Stream_Reply': {0x38, 0x39, 0x3A, 0x3B}, +} + + class MemorySpace(Enum): """The memory space to read. In practice, XMLDataProcessor (or a non-XML parser if necessary) @@ -105,8 +128,12 @@ class MemoryReadMemo: Attributes: data(bytearray): The data that was read. """ - def __init__(self, nodeID, size, space, address, rejectedReply, dataReply): + def __init__(self, nodeID: NodeID, size: int, space: int, address: int, + rejectedReply: Callable[['MemoryReadMemo'], None], + dataReply: Callable[['MemoryReadMemo'], None]): # For args see class docstring. + self.error = None # type: str|None + self.errorCode = None # type: int|None self.nodeID = nodeID self.size = size self.space = space @@ -116,6 +143,7 @@ def __init__(self, nodeID, size, space, address, rejectedReply, dataReply): # for convenience, data can be added or updated after creation of the # memo self.data = bytearray() + assertMemoOK(self) class MemoryWriteMemo: @@ -138,9 +166,13 @@ class MemoryWriteMemo: memory address. """ - def __init__(self, nodeID, okReply, rejectedReply, size, space, address, - data): + def __init__(self, nodeID: NodeID, + okReply: Callable[['MemoryWriteMemo'], None], + rejectedReply: Callable[['MemoryWriteMemo'], None], + size: int, space: int, address: int, data: bytearray): # For args see class docstring. + self.error = None # type: str|None + self.errorCode = None # type: int|None self.nodeID = nodeID self.okReply = okReply self.rejectedReply = rejectedReply @@ -148,6 +180,84 @@ def __init__(self, nodeID, okReply, rejectedReply, size, space, address, self.space = space self.address = address self.data = data + assertMemoOK(self) + + +def assertMemoOK(memo: Union[MemoryReadMemo, MemoryWriteMemo]): + assert isinstance(memo.space, int), \ + f"Expected int or MemorySpace.value, got space={emit_cast(memo.space)}" + assert isinstance(memo.size, int), \ + f"Expected int, got size={emit_cast(memo.size)}" + assert memo.size <= 64, \ + f"Expected <= 64, got size={memo.size}" + assert isinstance(memo.address, int), \ + f"Expected int, got address={emit_cast(memo.address)}" + assert isinstance(memo.data, Union[bytes, bytearray]), \ + f"Expected bytearray, got data={emit_cast(memo.data)}" + + +def parseReplyDatagram(memo: Union[MemoryReadMemo, MemoryWriteMemo], + dmemo: Union[DatagramReadMemo, DatagramWriteMemo]): + """Parse dmemo and set errorCode and/or error attributes of memo""" + if not dmemo.data or dmemo.data[0] != 0x20: + logger.warning( + "Datagram type is not memory configuration (0x20)" + f" it is {hex(dmemo.data[0])}") + return + if len(dmemo.data) < 2: + logger.warning( + "Datagram is truncated to 1 byte:" + f" it is {hex(dmemo.data[0])}") + return + (hasByte6, _) = Convert.deserializeMC2ndByte(dmemo.data[1]) + offset = 6 + error = None + if hasByte6: + offset = 7 + memo.error = None + memo.errorCode = None + if (dmemo.data[1] & 0x08 == 0): + # ok reply + return + else: + pass + # 0x08 (0b00001000) is error bit + # mode = None + # for k, values in MODE_ERROR_BYTES.items(): + # if dmemo.data[1] in values: + # mode = k + # break + # if mode is not None: + # error = f"No {mode} error code." + # else: + # error = f"No error code for unknown mode {hex(dmemo.data[1])}." + code_idx = offset + + if len(dmemo.data) < code_idx + 2: + memo.error = error + if len(dmemo.data) == code_idx + 1: + memo.error = ( + f"malformed error code {hex(dmemo.data[code_idx])}" + " (expected 2 bytes)") + memo.errorCode = dmemo.data[code_idx] + return + error = None + # Decode big-endian number: + memo.errorCode = dmemo.data[code_idx] << 8 + dmemo.data[code_idx+1] + message_idx = code_idx + 2 + if len(dmemo.data) > message_idx: + error_bytes = Convert.getBeforeNull(dmemo.data, message_idx) + error = error_bytes.decode("utf-8") + if len(error) == 1: + error += f" ({hex(dmemo.data[message_idx])})" + elif len(error) == 0 and (len(dmemo.data) - message_idx > 0): + error += f" ({list(dmemo.data[message_idx:])})" + else: + error = f"(2nd byte = {hex(dmemo.data[1])})" + error += f" (hasByte6={hasByte6})" + if hasByte6: + error += f" (space={hex(dmemo.data[6])})" + memo.error = error class MemoryService: @@ -196,15 +306,15 @@ def requestMemoryReadNext(self, memo, stream: bool = False): memo (MemoryReadMemo): Request to send. """ assert isinstance(stream, bool) - byte6 = False # if custom space is defined in byte 6 + hasByte6 = False # if custom space is defined in byte 6 flag = 0 - (byte6, flag) = Convert.spaceDecode(memo.space) + (hasByte6, flag) = Convert.serializeSpace(memo.space) if stream: # Encoding: 0x60=custom, 0x61=0xFD, 0x62=0xFE, 0x63=0xFF - spaceFlag = 0x60 if byte6 else (flag | 0x60) + spaceFlag = 0x60 if hasByte6 else (flag | 0x60) else: # Encoding: 0x40=custom, 0x41=0xFD, 0x42=0xFE, 0x43=0xFF - spaceFlag = 0x40 if byte6 else (flag | 0x40) # | 0b11111100 + spaceFlag = 0x40 if hasByte6 else (flag | 0x40) # | 0b11111100 # ^ In else case, flag is 1-3, so re-add 0xFC (0b11111100) addr2 = ((memo.address >> 24) & 0xFF) addr3 = ((memo.address >> 16) & 0xFF) @@ -214,7 +324,7 @@ def requestMemoryReadNext(self, memo, stream: bool = False): DatagramService.ProtocolID.MemoryOperation.value, spaceFlag, addr2, addr3, addr4, addr5]) # NOTE: list[int] is ok for bytearray extend (`+` requires cast) - if byte6: + if hasByte6: assert memo.space <= 0xFF, f"Space {memo.space} out of byte range" data.extend([(memo.space & 0xFF)]) data.extend([memo.size]) @@ -272,6 +382,7 @@ def datagramReceivedListener(self, dmemo: DatagramReadMemo) -> bool: if len(self.readMemos) > 0: self.requestMemoryReadNext(self.readMemos[0]) + parseReplyDatagram(tMemoryMemo, dmemo) # fill data for call-back to requestor if len(dmemo.data) > offset: tMemoryMemo.data = dmemo.data[offset:] @@ -295,6 +406,7 @@ def datagramReceivedListener(self, dmemo: DatagramReadMemo) -> bool: if self.writeMemos[index].nodeID == dmemo.srcID: writeMemo = self.writeMemos[index] # type: MemoryWriteMemo del self.writeMemos[index] + parseReplyDatagram(writeMemo, dmemo) if dmemo.data[1] & 0x08 == 0 : writeMemo.okReply(writeMemo) else: @@ -334,15 +446,15 @@ def requestMemoryWrite(self, memo: MemoryWriteMemo, stream: bool = False): # preserve the request self.writeMemos.append(memo) # create & send a write datagram - byte6 = False # if custom space is defined in byte 6 + hasByte6 = False # if custom space is defined in byte 6 flag = 0 - (byte6, flag) = Convert.spaceDecode(memo.space) + (hasByte6, flag) = Convert.serializeSpace(memo.space) if stream: # Encoding: 0x20=custom, 0x21=0xFD, 0x22=0xFE, 0x23=0xFF - spaceFlag = 0x20 if byte6 else (flag | 0x20) + spaceFlag = 0x20 if hasByte6 else (flag | 0x20) else: # Encoding: 0x00=custom, 0x01=0xFD, 0x02=0xFE, 0x03=0xFF - spaceFlag = 0x00 if byte6 else (flag | 0x00) + spaceFlag = 0x00 if hasByte6 else (flag | 0x00) addr2 = ((memo.address >> 24) & 0xFF) addr3 = ((memo.address >> 16) & 0xFF) addr4 = ((memo.address >> 8) & 0xFF) @@ -351,7 +463,7 @@ def requestMemoryWrite(self, memo: MemoryWriteMemo, stream: bool = False): DatagramService.ProtocolID.MemoryOperation.value, spaceFlag, addr2, addr3, addr4, addr5 ]) - if byte6: + if hasByte6: assert memo.space <= 0xFF, f"Space {memo.space} out of byte range" data.extend([(memo.space & 0xFF)]) data.extend(memo.data) @@ -365,7 +477,7 @@ def requestSpaceLength(self, space: int, nodeID: NodeID, Args: space (int): Encoded memory space identifier. This can be a value within a specific range, as defined in the - `spaceDecode` method. + `serializeSpace` method. nodeID (NodeID): ID of remote node from which the memory space length is requested. callback (Callable): Callback function that will receive the diff --git a/tests/test_convert.py b/tests/test_convert.py index e89b019..4271edd 100644 --- a/tests/test_convert.py +++ b/tests/test_convert.py @@ -87,18 +87,18 @@ def testIntToArrayFail(self): with self.assertRaises(ValueError): Convert.intToArray(value, length) - def testSpaceDecode(self): + def testSerializeSpace(self): byte6 = False space = 0x00 - (byte6, space) = Convert.spaceDecode(0xF8) + (byte6, space) = Convert.serializeSpace(0xF8) self.assertEqual(space, 0xF8) self.assertTrue(byte6) - (byte6, space) = Convert.spaceDecode(0xFF) + (byte6, space) = Convert.serializeSpace(0xFF) self.assertEqual(space, 0x03) self.assertFalse(byte6) - (byte6, space) = Convert.spaceDecode(0xFD) + (byte6, space) = Convert.serializeSpace(0xFD) self.assertEqual(space, 0x01) self.assertFalse(byte6) From b5a54dc710e17ae8abe8532501bf983eacc9f867 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 16:43:10 -0400 Subject: [PATCH 24/34] Allow length request reply to have arbitrary size. --- openlcb/memoryservice.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index 2b98224..98f9a15 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -188,10 +188,12 @@ def assertMemoOK(memo: Union[MemoryReadMemo, MemoryWriteMemo]): f"Expected int or MemorySpace.value, got space={emit_cast(memo.space)}" assert isinstance(memo.size, int), \ f"Expected int, got size={emit_cast(memo.size)}" - assert memo.size <= 64, \ - f"Expected <= 64, got size={memo.size}" + # TODO: > 64 is only ok for a length request (?) + # assert memo.size <= 64, \ + # f"Expected <= 64, got size={memo.size}" assert isinstance(memo.address, int), \ f"Expected int, got address={emit_cast(memo.address)}" + assert len(memo.data) <= 64 assert isinstance(memo.data, Union[bytes, bytearray]), \ f"Expected bytearray, got data={emit_cast(memo.data)}" From 0074507377ec8344969008378539912d8a127bba Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 17:12:25 -0400 Subject: [PATCH 25/34] Fix error code deserialization (order of operation issue; result now matches JMRI). --- openlcb/memoryservice.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index 98f9a15..f6c2cd9 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -245,7 +245,7 @@ def parseReplyDatagram(memo: Union[MemoryReadMemo, MemoryWriteMemo], return error = None # Decode big-endian number: - memo.errorCode = dmemo.data[code_idx] << 8 + dmemo.data[code_idx+1] + memo.errorCode = (dmemo.data[code_idx] << 8) + dmemo.data[code_idx+1] message_idx = code_idx + 2 if len(dmemo.data) > message_idx: error_bytes = Convert.getBeforeNull(dmemo.data, message_idx) From 1f9afcd4d12e1da3255e46468e64368bd673b43a Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Fri, 1 May 2026 17:12:32 -0400 Subject: [PATCH 26/34] Add debug output for unmatched datagrams. --- openlcb/datagramservice.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/openlcb/datagramservice.py b/openlcb/datagramservice.py index 743cc21..ff412f4 100644 --- a/openlcb/datagramservice.py +++ b/openlcb/datagramservice.py @@ -247,7 +247,11 @@ def handleDatagramReceivedOK(self, message: Message): memo = self.matchToWriteMemo(message) # type: DatagramWriteMemo|None # check for whether a match was found, indicating this was for us - if memo is None : return + if memo is None: + logger.debug( + f"Unrelated OK reply discarded: from" + f" {message.source} to {message.destination}") + return # check of tracking logic if self.currentOutstandingMemo != memo: @@ -269,6 +273,9 @@ def handleDatagramRejected(self, message: Message): # check for whether a match was found, indicating this was for us if memo is None: + logger.debug( + f"Unrelated Rejected reply discarded: from" + f" {message.source} to {message.destination}") return # check of tracking logic From c4f69bd812f5477e0fc58a6a09f65b2871d7dce5 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Tue, 5 May 2026 15:43:46 -0400 Subject: [PATCH 27/34] Track whether CDI was loaded from cache (file). Add assert to clarify _space value instead of causing unclear downstream error. --- openlcb/dataprocessor.py | 8 ++++++++ openlcb/xmldataprocessor.py | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/openlcb/dataprocessor.py b/openlcb/dataprocessor.py index 660e350..f3f7109 100644 --- a/openlcb/dataprocessor.py +++ b/openlcb/dataprocessor.py @@ -17,8 +17,16 @@ class DataProcessor: DEFAULT_EXT = ".bin" # override in subclass def __init__(self): + self._is_from_cache = False # type: bool + self._path = None # type: str|None self.enable_cache = False # type: bool # Members used to construct space memo such as CDIMemo: self.progress_ratio = None # type: float|None self.progress_count = None # type: int|None self.expected_size = None # type: int|None + + def getPath(self): + return self._path + + def isFromCache(self): + return self._is_from_cache diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index bb0be8d..ed34fa4 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -316,6 +316,7 @@ def load(self, node_id: NodeID, path, space: Union[MemorySpace, int], format: Union[DataFormat, None] = None): """Load instead of downloading.""" assert not self._data + self._is_from_cache = True self.onStartDownload() assert isinstance(space, (MemorySpace, int)) if isinstance(space, int): @@ -336,6 +337,7 @@ def load(self, node_id: NodeID, path, space: Union[MemorySpace, int], data = None with open(path, "rb") as stream: data = stream.read() # type:ignore + self._path = path if self._format is DataFormat.XML: if memo is not None: assert isinstance(memo, MemoryReadMemo) @@ -351,8 +353,10 @@ def memoryReadFail(memo: MemoryReadMemo): assert data is not None # Based on _startMemoryRead in OpenLCBNetwork: + _space = self.getSpaceValue() # self._space is set above + assert _space is not None memo = MemoryReadMemo(node_id, len(data), - self.getSpaceValue(), 0, + _space, 0, memoryReadFail, memoryReadSuccess) assert data is not None From c2df1850bd669348ac965d9a75454fc9b22f962d Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Tue, 5 May 2026 17:48:58 -0400 Subject: [PATCH 28/34] Fix: Respect cache setting (Only save CDI file in that case). --- openlcb/xmldataprocessor.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index ed34fa4..ec9a4e3 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -439,12 +439,13 @@ def _feedLast(self, memo: MemoryReadMemo, enable_cache=None): if self._realtime: self._parser.feed(partial_str) # may call startElement/endElement # memo = MemoryReadMemo(memo) - path = self.cacheFilePath(memo.nodeID) - with open(path, 'w') as stream: - if cdiString is None: - cdiString = self._data.rstrip(b'\0').decode("utf-8") - stream.write(cdiString) - print('Saved {}'.format(repr(path))) + if enable_cache: + path = self.cacheFilePath(memo.nodeID) + with open(path, 'w') as stream: + if cdiString is None: + cdiString = self._data.rstrip(b'\0').decode("utf-8") + stream.write(cdiString) + print('Saved {}'.format(repr(path))) self._data = None # Ensure isn't reused for more than one doc def cacheFilePathCustom(self, item_id: Union[NodeID, str], **kwargs): From 8b687db6e295f5a94ae86d5af1188071b6c8cac3 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Mon, 11 May 2026 16:18:20 -0400 Subject: [PATCH 29/34] Fix non-subscripable type (Change to PEP8 commented type hint). --- openlcb/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openlcb/__init__.py b/openlcb/__init__.py index 325ed0f..ca124dc 100644 --- a/openlcb/__init__.py +++ b/openlcb/__init__.py @@ -23,11 +23,14 @@ ORD_z = 0x7A -def only_hex_pairs(value: str) -> Union[re.Match[bytes], re.Match[str], None]: +def only_hex_pairs(value: str): + # type: (str) -> Union[re.Match[bytes], re.Match[str], None] """Check if string contains only machine-readable hex pairs. See openlcb.conventions submodule for LCC ID dot notation functions (less restrictive). """ + # ^ PEP8 (instead of Python) type hint is used to avoid + # "TypeError: 'type' object is not subscriptable" if isinstance(value, (bytearray, bytes)): return hex_pairs_brc.fullmatch(value) assert isinstance(value, str) From bcef09b40c09ef940663e1d76e774918dd54d75c Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Mon, 11 May 2026 16:19:03 -0400 Subject: [PATCH 30/34] Add clear option such as for reconnect if client code doesn't want to retain the remote node list. --- openlcb/nodestore.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/openlcb/nodestore.py b/openlcb/nodestore.py index cb423ba..498e8b5 100644 --- a/openlcb/nodestore.py +++ b/openlcb/nodestore.py @@ -38,6 +38,10 @@ def store(self, node: Node) : self.nodes.sort(key=lambda x: x.snip.userProvidedNodeName, reverse=True) + def clear(self): + self.byIdMap.clear() + self.nodes.clear() + def isPresent(self, nodeID: NodeID) -> bool: return self.byIdMap.get(nodeID) is not None From 1d6b3ff75d8f204a6c479f067989de4d2d1b7fb9 Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Tue, 19 May 2026 14:05:48 -0400 Subject: [PATCH 31/34] (NOOP) Fix some type hints and a docstring. --- openlcb/__init__.py | 5 +++++ openlcb/memoryservice.py | 2 +- openlcb/xmldataprocessor.py | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/openlcb/__init__.py b/openlcb/__init__.py index ca124dc..dc8ae02 100644 --- a/openlcb/__init__.py +++ b/openlcb/__init__.py @@ -140,6 +140,7 @@ def from_all_hex_bytes(b: bytearray) -> bytearray: def hr_repr(value, always_quote: bool = False) -> str: """Represent value with double quotes + if str, otherwise as an unquoted string. (Human-readable repr). """ # repr_value = repr(value) @@ -153,4 +154,8 @@ def hr_repr(value, always_quote: bool = False) -> str: def d_quote(value) -> str: + """Represent any type of value in double-quotes + (with any already present escaped) such as for emitting XML + attribute debug messages or any other technical/literary use. + """ return hr_repr(value, always_quote=True) diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index f6c2cd9..a65ad14 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -281,7 +281,7 @@ def __init__(self, service: DatagramService): self.datagramReceivedListener ) - def requestMemoryRead(self, memo, stream: Bool = False): + def requestMemoryRead(self, memo, stream: bool = False): # type: (MemoryReadMemo, Optional[bool]) -> None '''Request a read operation start. diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index ec9a4e3..8c0ed7a 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -158,7 +158,7 @@ def __init__(self, linkLayer: CanLink, space: MemorySpace): # endregion ContentHandler self.acdi = False - def getRootMemo(self): + def getRootMemo(self) -> Union[CDIMemo, None]: """Get the root memo object if any. This should only be called after the entire file is parsed such as when cm.done is True in onStatusMemo(cm) callback. Set From d8b06807ba0676099b3c23e20c2ed2a67a34074f Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Tue, 19 May 2026 18:14:15 -0400 Subject: [PATCH 32/34] Begin implementing local node memory (FIXME: responding to memory configuration protocol from another node is not implemented). --- examples/example_node_implementation.py | 6 +- .../example_node_memory_implementation.py | 248 ++++++++++++++++++ openlcb/__init__.py | 14 + openlcb/cdimemo.py | 6 +- openlcb/cdivar.py | 3 +- openlcb/localnode.py | 227 ++++++++++++++++ openlcb/memoryservice.py | 2 +- openlcb/xmldataprocessor.py | 43 ++- 8 files changed, 536 insertions(+), 13 deletions(-) create mode 100644 examples/example_node_memory_implementation.py create mode 100644 openlcb/localnode.py diff --git a/examples/example_node_implementation.py b/examples/example_node_implementation.py index 768f09f..b9e9416 100644 --- a/examples/example_node_implementation.py +++ b/examples/example_node_implementation.py @@ -79,7 +79,11 @@ def printMessage(message): print("RM: {} from {}".format(message, message.source)) -canLink = CanLink(physicalLayer, NodeID(settings['localNodeID'])) +localNodeID = NodeID(settings['localNodeID']) +print() +print(f"[example_node_memory_implementation] localNodeID: {localNodeID}") + +canLink = CanLink(physicalLayer, localNodeID) canLink.registerMessageReceivedListener(printMessage) datagramService = DatagramService(canLink) diff --git a/examples/example_node_memory_implementation.py b/examples/example_node_memory_implementation.py new file mode 100644 index 0000000..d5011c1 --- /dev/null +++ b/examples/example_node_memory_implementation.py @@ -0,0 +1,248 @@ +''' +Demo of creating a virtual node to represent the application +(other local nodes are possible, but at least one is necessary +for the application to announce itself and provide SNIP info), +in this case with memory to allow another node to change settings +(could also be used to for a second virtual node such as to +represent/emulate a non-LCC train, but a separate +virtual node from the Configuration Tool is recommended in +that case). + +based on example_node_implementation from python-openlcb examples. + +Usage: +python3 example_node_memory_implementation.py [host|host:port] + +Options: +host|host:port (optional) Set the address (or using a colon, + the address and port). Defaults to a hard-coded test + address and port. +''' +import os +import socket +import struct + +# region same code as other examples +from examples_settings import Settings +from openlcb.localnode import LocalNode # do 1st to fix path if no pip install +settings = Settings() + +if __name__ == "__main__": + settings.load_cli_args(docstring=__doc__) +# endregion same code as other examples + +from openlcb import emit_cast, get_config_dir, precise_sleep # noqa: E402 +from openlcb.tcplink.tcpsocket import TcpSocket # noqa: E402 + +from openlcb.canbus.canphysicallayergridconnect import ( # noqa: E402 + CanPhysicalLayerGridConnect, +) +from openlcb.canbus.canlink import CanLink # noqa: E402 +from openlcb.nodeid import NodeID # noqa: E402 +from openlcb.datagramservice import DatagramService # noqa: E402 +from openlcb.memoryservice import MemoryService # noqa: E402 +from openlcb.message import Message # noqa: E402 +from openlcb.mti import MTI # noqa: E402 + +from openlcb.localnodeprocessor import LocalNodeProcessor # noqa: E402 +from openlcb.pip import PIP # noqa: E402 +from openlcb.snip import SNIP # noqa: E402 +from openlcb.node import Node # noqa: E402 + +# specify connection information +# region moved to settings +# host = "192.168.16.212" +# port = 12021 +# localNodeID = "05.01.01.01.03.01" +# farNodeID = "09.00.99.03.00.35" +# endregion moved to settings + +sock = TcpSocket() +# s.settimeout(30) +try: + sock.connect(settings['host'], settings['port']) +except socket.gaierror: + print("Failure accessing {}:{}" + .format(settings.get('host'), settings.get('port'))) + raise + +print("RR, SR are raw socket interface receive and send;" + " RL, SL are link interface; RM, SM are message interface") + + +# def sendToSocket(frame: CanFrame): +# string = frame.encodeAsString() +# print(" SR: {}".format(string.strip())) +# sock.sendString(string) +# physicalLayer.onFrameSent(frame) + + +def printFrame(frame): + print(" RL: {}".format(frame)) + + +physicalLayer = CanPhysicalLayerGridConnect() +physicalLayer.registerFrameReceivedListener(printFrame) + + +def printMessage(message): + print("RM: {} from {}".format(message, message.source)) + + +localNodeID = NodeID(settings['localNodeID']) +print() +print(f"[example_node_memory_implementation] localNodeID: {localNodeID}") +canLink = CanLink(physicalLayer, localNodeID) +canLink.registerMessageReceivedListener(printMessage) + +datagramService = DatagramService(canLink) +canLink.registerMessageReceivedListener(datagramService.process) + +spaces = { # big endian (most significant byte sent first) as per openlcb + # 0: bytearray([ + # 0x01, 0x00, # 0x1000 = 4096 (unsigned int 16) + # ]) + 0: bytearray(struct.pack(">H", 12021)), +} +# bytearray allows in-place append (from pack bytes does not) +# H: short (capitalized means unsigned) +# >: big endian (required for openlcb) +# e: float16 (IEEE 754 binary16, 2-bytes) +# For other symbols see Python documentation or SUBTYPE_FORMATS in cdivar.py. + +spaces[0] += struct.pack(">e", 0.5) # save at address 3 (size 2) +# NOTE: 0.5 can be stored precisely, as b'\x008' +# but not all numbers can be represented by IEEE float. +# For example, 2.4 is stored as b'\xcd@' which is ~2.400390625 + +# Additional pack examples: +neg2_float_ba = bytearray(b'\xc0\x00') +neg2_float_b = struct.pack(">e", -2) +assert bytes(neg2_float_ba) == bytes(neg2_float_b), \ + f"expected b'\xc0\x00', b'\xc0\x00', got {neg2_float_ba}, {neg2_float_b}" + +cdi = """ + + + python-openlcb example authors + example_node_memory_implementation + 1.0 + 1.0 + + + + + Port + Network port of remote hub (2-byte unsigned short) + 12021 + + + Timeout + Network timeout (2-byte binary16 value). + 0.5 + + + +""" # noqa: E501 + + +def handleDatagram(memo): + """create a call-back to print datagram contents when received + + Args: + memo (DatagramReadMemo): The datagram received + + Returns: + bool: Always False (True would mean we sent a reply to the datagram, + but let the MemoryService do that). + """ + print(f"Datagram receive call back: {emit_cast(memo)}") + return False + + +datagramService.registerDatagramReceivedListener(handleDatagram) + +memoryService = MemoryService(datagramService) + + +# callbacks to get results of memory read + +def memoryReadSuccess(memo): + print("successful memory read: {}".format(memo.data)) + + +def memoryReadFail(memo): + print("memory read failed: {}".format(memo.data)) + + +# create a node and connect it update +# This is a very minimal node, which just takes part in the low-level common +# protocols +localNode = LocalNode( + NodeID(settings['localNodeID']), + SNIP("python-openlcb example authors", + "example_node_memory_implementation", + "1.0", "1.0", "Custom Name Here", "Custom Description Here"), + set([ + PIP.SIMPLE_NODE_IDENTIFICATION_PROTOCOL, + PIP.DATAGRAM_PROTOCOL, + PIP.CONFIGURATION_DESCRIPTION_INFORMATION, + PIP.ADCDI_PROTOCOL, + PIP.MEMORY_CONFIGURATION_PROTOCOL, + ]), + canLink +) +my_conf_dir = os.path.join(get_config_dir("python-openlcb")) +backup_name = "example_node_memory_implementation.cdi.xml" +backup_path = os.path.join(my_conf_dir, backup_name) + +localNode.loadCDIString(cdi, backup_path) + +# localNodeProcessor = LocalNodeProcessor(canLink, localNode) +# canLink.registerMessageReceivedListener(localNodeProcessor.process) +localNodeProcessor = localNode.localNodeProcessor + + +def displayOtherNodeIds(message) : + """Listener to identify connected nodes + + Args: + message (Message): A response from the network + """ + print("[displayOtherNodeIds] type(message): {}" + "".format(type(message).__name__)) + if message.mti == MTI.Verified_NodeID : + print("Detected farNodeID is {}".format(message.source)) + + +canLink.registerMessageReceivedListener(displayOtherNodeIds) + + +####################### + +# have the socket layer report up to bring the link layer up and get an alias + +print(" SL : link up...") +physicalLayer.physicalLayerUp() +print(" SL : link up...waiting...") +while canLink.pollState() != CanLink.State.Permitted: + physicalLayer.receiveAll(sock, verbose=settings['trace']) + physicalLayer.sendAll(sock, verbose=True) + precise_sleep(.02) +print(" SL : link up") +# request that nodes identify themselves so that we can print their node IDs +message = Message(MTI.Verify_NodeID_Number_Global, + NodeID(settings['localNodeID']), None) +canLink.sendMessage(message) + +# process resulting activity +while True: + count = 0 + count += physicalLayer.sendAll(sock, verbose=True) + count += physicalLayer.receiveAll(sock, verbose=settings['trace']) + if count < 1: + precise_sleep(.01) + # else skip sleep to avoid latency (port already delayed) + +physicalLayer.physicalLayerDown() diff --git a/openlcb/__init__.py b/openlcb/__init__.py index dc8ae02..3e7a8ac 100644 --- a/openlcb/__init__.py +++ b/openlcb/__init__.py @@ -1,4 +1,6 @@ from enum import Enum +import os +import platform import re import time @@ -23,6 +25,18 @@ ORD_z = 0x7A +def get_config_dir(unique_software_name: str): + """Get a configuration directory for any program + (In the parent directory recommended by the specific platform). + """ + CONFIGS = os.path.expanduser("~/.config") + if platform.system() == "Darwin": + CONFIGS = os.path.expanduser("~/Library/Application Support") + elif platform.system() == "Windows": + CONFIGS = os.environ['APPDATA'] + return os.path.join(CONFIGS, unique_software_name) + + def only_hex_pairs(value: str): # type: (str) -> Union[re.Match[bytes], re.Match[str], None] """Check if string contains only machine-readable hex pairs. diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index 9803d1f..5ef49b1 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -233,10 +233,12 @@ def toCDIVar(self): result.signed = True # if self.size is not None: if result.size not in [1, 2, 4, 8]: + children_msg = json.dumps(self.children, sort_keys=True, + indent=2, + default=CDIMemo.to_dict) raise AttributeError( f"expected 1,2,4,8 for int size, got {result.size}" - f" in children={json.dumps(self.children, sort_keys=True, indent=2, - default=CDIMemo.to_dict)}") + f" in children={children_msg}") if result.max is None: if result.signed: result.max = math.pow(2, result.size * 8 - 1) - 1 diff --git a/openlcb/cdivar.py b/openlcb/cdivar.py index 3a51db9..26c374a 100644 --- a/openlcb/cdivar.py +++ b/openlcb/cdivar.py @@ -195,7 +195,8 @@ def setInt(self, value: int): self.data = self.intToData(value) def floatToData(self, value: float) -> bytes: - assert self.className == "float" + assert self.className == "float", \ + f"floatToData attempted on non-float: {self.className}" assert isinstance(value, float) return struct.pack(self.packFormat(), value) diff --git a/openlcb/localnode.py b/openlcb/localnode.py new file mode 100644 index 0000000..8f44a29 --- /dev/null +++ b/openlcb/localnode.py @@ -0,0 +1,227 @@ +import os + +from logging import getLogger +from typing import Union +from openlcb import emit_cast +from openlcb.node import PIP, SNIP, Node + +from openlcb.localnodeprocessor import LocalNodeProcessor +from openlcb.nodeid import ( + NodeID, +) +from openlcb.xmldataprocessor import ( + CanLink, + CDIMemo, + CDIVar, + # CLASSNAME_TYPES, + d_quote, + MemoryReadMemo, + MemorySpace, + XMLDataProcessor, +) + +logger = getLogger(__name__) + + +class LocalNode(Node): + """A Node with its own virtual memory + (emulate memory spaces such as for creating a virtual + signal node with settings)""" + def __init__(self, id: NodeID, snip: SNIP, pipSet: set, + linkLayer: CanLink): + Node.__init__(self, id, snip, pipSet) + self.cdi = None # type: XMLDataProcessor|None + self._replicated_cdi_tree = None # type: CDIMemo|None + if PIP.CONFIGURATION_DESCRIPTION_INFORMATION in pipSet: + self.cdi = XMLDataProcessor(linkLayer, MemorySpace.CDI) + else: + logger.warning( + "PIP.CONFIGURATION_DESCRIPTION_INFORMATION is not in pipSet" + f" for new LocalNode {self.cdi}, so XMLDataProcessor" + " will not be initialized (functioning as Node, unless" + " remote user knows addresses apart from CDI)") + self.spaces = {} # type: dict[int, bytearray] + self.localNodeProcessor = LocalNodeProcessor(linkLayer, self) + linkLayer.registerMessageReceivedListener( + self.localNodeProcessor.process) + + def loadCDIFile(self, path, memo=None): + """Load a CDI file to generate virtual memory spaces + (to create a virtual node, not representing a remote one) + + Args: + path (str): Location of original file, also used + to generate cache dir (parent of path). + memo (MemoryReadMemo): Typically left blank, + This would provide a success or fail message, + but this method can be called asynchronously + since LocalNode assumes local data is loaded, + not network data. + """ + assert isinstance(path, str) + if not os.path.isfile(path): + raise FileNotFoundError(path) + + # self.cdi.load(self.id, path, MemorySpace.CDI, memo) + xml_data = None + with open(path, "wb") as stream: + xml_data = stream.read() + return self.loadCDIString(xml_data, path, memo=memo) + + def loadCDIString(self, xml_data, path, memo=None): + """Load raw XML data from a string. + Args: + xml_data (Union[bytes, bytearray, str]): Raw XML + path (str): Location of original file, for + reference and use as cache dir (parent of path). + memo (Optional[MemoryReadMemo]): Typically left blank, + This would provide a success or fail message, + but this method can be called asynchronously + since LocalNode assumes local data is loaded, + not network data. + """ + self.cdiBackupDir = os.path.dirname(path) + assert self.cdi is not None, \ + ("PIP.CONFIGURATION_DESCRIPTION_INFORMATION is not in pipSet" + f" for LocalNode {self.id}") + if memo is None: + memo = MemoryReadMemo( + self.id, 0, MemorySpace.CDI.value, 0, + self.onCDILoadFailed, self.onCDILoaded) + self.cdi.load(self.id, path, MemorySpace.CDI, memo, data=xml_data) + # with open(path, "r") as stream: + # data = stream.read() + # self.tree = etree.fromstring(data) + self.reserveSpaces() + + def setMemory(self, memo: CDIMemo, var: CDIVar): + """Set a memory address at memo to the value in var""" + assert memo.space is not None + size = memo.getSize() + assert size is not None + assert size > 0, f"size={repr(size)}" + assert memo.address is not None + # if var is None: + # var = memo.toCDIVar() + assert var is not None + assert var.data is not None + assert len(var.data) == memo.getSize() + self.setMemoryAt(memo.space, memo.address, var.data) + + def setMemoryAt(self, space, address, data): + """Set address in virtual memory space to data""" + assert isinstance(data, (bytearray, bytes)) + if isinstance(space, MemorySpace): + space = space.value + assert isinstance(space, int) + assert isinstance(address, int) + assert address >= 0 + size = len(data) + end = address + size + + if space not in self.spaces: + self.spaces[space] = bytearray() + else: + assert isinstance(self.spaces[space], bytearray) + + newRegionLen = end - len(self.spaces[space]) + if newRegionLen > 0: + logger.warning( + f"Extending LocalNode data from {len(self.spaces[space])}" + f" byte(s) to {end} byte(s).") + self.spaces[space] += b'\0' * newRegionLen + assert end - address == len(data) + self.spaces[space][address:end] = data + print(f"Set LocalNode {self.id} space {space}" + f" address {space} (length {len(data)}).") + + def reserveSpaces(self, parent: Union[CDIMemo, None] = None): + + assert self.cdi is not None, \ + ("PIP.CONFIGURATION_DESCRIPTION_INFORMATION is not in pipSet" + f" for LocalNode {self.id}") + + if parent is None: + parent = self.cdi.getRootMemo() + assert parent is not None + assert parent.tag == "cdi", f"Expected cdi, got {parent.tag}" + assert parent.element is not None + if (('replicated' in parent.element.attrib) + and (parent.element.attrib['replicated'] == "true")): + # Caller already used expandedTree + return self._reserveSpaces(parent=parent) + expanded_root_memo, expanded_root = self.cdi.expandedTree() + self.cdi.expanded_root_memo = expanded_root_memo + self.cdi.expanded_root = expanded_root + # ^ self.cdi.expanded_root_memo can also be set via + # self.cdi.extractCDIVarMemos. + return self._reserveSpaces( + parent=self.cdi.expanded_root_memo, + ) + + def _reserveSpaces(self, parent: Union[CDIMemo, None] = None, level=0): + assert parent is not None + assert parent.tag is not None + tag = parent.tag.lower() + if tag == "cdi": + assert parent.element is not None + assert parent.element.attrib['replicated'] == "true", \ + "expanded_root_memo accounting for replication must be used." + if tag in ("int", "float"): # CLASSNAME_TYPES: + # cast_fn = int if tag == "int" else float + var = parent.toCDIVar() + # _min = parent.getChildContentN("min", tag) + # _max = parent.getChildContentN("max", tag) + value = parent.getChildContentN("default", tag) + # size = parent.getSize() + # assert size is not None + # var = CDIVar(parent.tag, _min=_min, _max=_max, _size=size) + if value is not None: + if tag == "float": + var.setFloat(value) + elif tag == "int": + assert isinstance(value, int), \ + f"tried to use {emit_cast(value)} for int tag" + var.setInt(value) + assert var.data is not None + # var.default = bytearray(copy.deepcopy(var.data)) + assert parent.space is not None, \ + f"No space defined in CDI for a(n) {tag}" + self.setMemory(parent, var) + return + for child in parent.children: + self._reserveSpaces(child, level=level+1) + if level == 0: + if not self.cdiBackupDir: + logger.warning( + f"Not backing up virtual node {self.id} memory since" + " no cdiBackupDir is set for the LocalNode instance.") + return + if not os.path.isdir(self.cdiBackupDir): + logger.warning( + f"Creating cdiBackupDir {self.cdiBackupDir}") + os.makedirs(self.cdiBackupDir) + for space, data in self.spaces.items(): + name = f"{self.id}.lcc-link-virtual-node.space={space}.xml" + path = os.path.join(self.cdiBackupDir, name) + with open(path, "wb") as stream: + stream.write(data) + print(f"Wrote {d_quote(path)}") + + def onCDILoaded(self, memo: MemoryReadMemo): + """Default handler, typically enough since CDI is local + in the case of LocalNode""" + assert self.cdi is not None, \ + ("PIP.CONFIGURATION_DESCRIPTION_INFORMATION is not in pipSet" + f" for LocalNode {self.id}") + print(f"LocalNode onFileLoaded {self.cdi.getPath()}: {memo}") + + def onCDILoadFailed(self, memo: MemoryReadMemo): + """Default handler for file load failed. + Shouldn't happen unless application has provided malformed XML, + since CDI is local in the case of LocalNode + """ + assert self.cdi is not None, \ + ("PIP.CONFIGURATION_DESCRIPTION_INFORMATION is not in pipSet" + f" for LocalNode {self.id}") + print(f"LocalNode onCDILoadFailed {self.cdi.getPath()}: {memo}") diff --git a/openlcb/memoryservice.py b/openlcb/memoryservice.py index a65ad14..997706b 100644 --- a/openlcb/memoryservice.py +++ b/openlcb/memoryservice.py @@ -194,7 +194,7 @@ def assertMemoOK(memo: Union[MemoryReadMemo, MemoryWriteMemo]): assert isinstance(memo.address, int), \ f"Expected int, got address={emit_cast(memo.address)}" assert len(memo.data) <= 64 - assert isinstance(memo.data, Union[bytes, bytearray]), \ + assert isinstance(memo.data, (bytes, bytearray)), \ f"Expected bytearray, got data={emit_cast(memo.data)}" diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 8c0ed7a..4e638ee 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -313,8 +313,18 @@ def _feedNext(self, memo: MemoryReadMemo): def load(self, node_id: NodeID, path, space: Union[MemorySpace, int], memo: Union[MemoryReadMemo, None] = None, - format: Union[DataFormat, None] = None): - """Load instead of downloading.""" + format: Union[DataFormat, None] = None, + data: Union[bytes, bytearray, str, None] = None): + """Load instead of downloading. + Args: + path (str): Location of original xml data (unused if + data is specified, but may be used for tracing; + Always sets self._path). + data (Optional[Union[bytes, bytearray, str]]): Actual XML data, + optional if path exists. If None, + path will be loaded, otherwise it will not, + and data will be used instead. + """ assert not self._data self._is_from_cache = True self.onStartDownload() @@ -334,9 +344,13 @@ def load(self, node_id: NodeID, path, space: Union[MemorySpace, int], self._format = format self._space = space # type:ignore # int if device-specific logger.warning(f"Using device-specific space: {space}") - data = None - with open(path, "rb") as stream: - data = stream.read() # type:ignore + if data is None: + with open(path, "rb") as stream: + data = stream.read() # type:ignore + else: + if isinstance(data, str): + # Mimic network data by converting to bytes: + data = data.encode('utf-8') self._path = path if self._format is DataFormat.XML: if memo is not None: @@ -543,6 +557,9 @@ def startElement(self, name: str, if self._tag_stack: parent_cm = self._tag_stack[-1] cm = CDIMemo(tag=name, element=el, parent=parent_cm, document=self) + # cm.space = self._tmp_space Commented since not replicated! + # - address and space should be set by expandedTree or the + # node processing the CDI, accounting for replication. if name == "segment": self._tmp_space = attrib.get('space') self._tmp_address = int(attrib.get('origin', 0)) @@ -705,12 +722,22 @@ def expandedTree(self) -> Tuple[CDIMemo, ET.Element]: new_root = ET.Element("cdi") # always new: children added from memos new_root.attrib.update(root_memo.element.attrib) + new_root.attrib['replicated'] = "true" + if new_root.tag != "cdi": + logger.warning( + f"expected cdi got {new_root.tag} from {root_memo.tag}") - new_root_memo = copy.deepcopy(root_memo) # deepcopy to edit children! + tmp_el = root_memo.element + root_memo.element = None # avoid deepcopy. Temporarily erase it. + new_root_memo = copy.deepcopy(root_memo) # copy to avoid affecting old + root_memo.element = tmp_el # restore the old copy. new_root_memo.document = self - size = self._expanded_tree_recursive(new_root_memo, new_root, address=0) + new_root_memo.element = new_root # use this not root_memo.element copy + size = self._expanded_tree_recursive(new_root_memo, new_root, + address=0) if size < 1: - logger.warning(f"No space used by CDI after replication (size={size})") + logger.warning( + f"No space used by CDI after replication (size={size})") return new_root_memo, new_root def _expanded_tree_recursive( From 82326de86743f72e96f31d382d431f735bd05e0a Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Tue, 19 May 2026 18:21:03 -0400 Subject: [PATCH 33/34] (NOOP) Rename expanded* to replicated* for consistency. --- examples/example_cdi_access.py | 2 +- openlcb/cdimemo.py | 6 ++-- openlcb/localnode.py | 14 ++++----- openlcb/xmldataprocessor.py | 56 +++++++++++++++++----------------- 4 files changed, 39 insertions(+), 39 deletions(-) diff --git a/examples/example_cdi_access.py b/examples/example_cdi_access.py index 4bbf5f2..c27b75b 100644 --- a/examples/example_cdi_access.py +++ b/examples/example_cdi_access.py @@ -191,7 +191,7 @@ class MyHandler(xml.sax.handler.ContentHandler): This is implementation-specific, and not required if streaming (parser.feed). _tmp_address (int|None): For sanity check, not actual address. - See expandedTree docstring. + See replicatedTree docstring. """ def __init__(self): diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index 5ef49b1..ba6ad9b 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -47,7 +47,7 @@ class CDIMemo(DataProcessorMemo): name (str): Name (determined by `name` child element content). space (int|None): The memory space address (May be one in MemorySpace values, or not if vendor-specific such as - defined in CDI etc. See expandedTree in XMLDataProcessor). + defined in CDI etc. See replicatedTree in XMLDataProcessor). stray (bool): The end tag is misplaced (doesn't match a start tag) due to bad xml or incorrect parsing. tail (str|None): Content following the end tag (not used in @@ -188,7 +188,7 @@ def toCDIVar(self): See LCC "Configuration Description Information" Standard. NOTE: The `address` is only correct if this CDIMemo has been - replicated (such as in expandedTree or self.expanded_root). + replicated (such as in replicatedTree or self.replicated_root). """ # result = CDIVar(self.tag) assert (self.tag is not None) and (self.tag.strip()) @@ -219,7 +219,7 @@ def toCDIVar(self): # enforces size: result = CDIVar(self.tag, _min=result_min, _max=result_max, _size=result_size, _default=result_default) - result.address = self.address # only set in expandedTree() + result.address = self.address # only set in replicatedTree() result.space = self.space result.floatFormat = result_floatFormat result.name = self.getChildContent("name") diff --git a/openlcb/localnode.py b/openlcb/localnode.py index 8f44a29..8053085 100644 --- a/openlcb/localnode.py +++ b/openlcb/localnode.py @@ -148,15 +148,15 @@ def reserveSpaces(self, parent: Union[CDIMemo, None] = None): assert parent.element is not None if (('replicated' in parent.element.attrib) and (parent.element.attrib['replicated'] == "true")): - # Caller already used expandedTree + # Caller already used replicatedTree return self._reserveSpaces(parent=parent) - expanded_root_memo, expanded_root = self.cdi.expandedTree() - self.cdi.expanded_root_memo = expanded_root_memo - self.cdi.expanded_root = expanded_root - # ^ self.cdi.expanded_root_memo can also be set via + replicated_root_memo, replicated_root = self.cdi.replicatedTree() + self.cdi.replicated_root_memo = replicated_root_memo + self.cdi.replicated_root = replicated_root + # ^ self.cdi.replicated_root_memo can also be set via # self.cdi.extractCDIVarMemos. return self._reserveSpaces( - parent=self.cdi.expanded_root_memo, + parent=self.cdi.replicated_root_memo, ) def _reserveSpaces(self, parent: Union[CDIMemo, None] = None, level=0): @@ -166,7 +166,7 @@ def _reserveSpaces(self, parent: Union[CDIMemo, None] = None, level=0): if tag == "cdi": assert parent.element is not None assert parent.element.attrib['replicated'] == "true", \ - "expanded_root_memo accounting for replication must be used." + "replicated_root_memo accounting for replication must be used." if tag in ("int", "float"): # CLASSNAME_TYPES: # cast_fn = int if tag == "int" else float var = parent.toCDIVar() diff --git a/openlcb/xmldataprocessor.py b/openlcb/xmldataprocessor.py index 4e638ee..541a048 100644 --- a/openlcb/xmldataprocessor.py +++ b/openlcb/xmldataprocessor.py @@ -110,7 +110,7 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): _tmp_space (int|None): What space we are currently on (of data described by Element(s), not of XML data itself). _tmp_address (int|None): For sanity check, not actual address - (no replication)! See expandedTree docstring. + (no replication)! See replicatedTree docstring. """ XML_TOP_TAGS = ("cdi", "fdi") DEFAULT_EXT = ".cdi.xml" # override in subclass @@ -120,8 +120,8 @@ class XMLDataProcessor(xml.sax.handler.ContentHandler, DataProcessor): def __init__(self, linkLayer: CanLink, space: MemorySpace): self.canLink: CanLink = linkLayer # caches_dir = SysDirs.Cache - self.expanded_root = None # type: ET.Element|None - self.expanded_root_memo = None # type: CDIMemo|None + self.replicated_root = None # type: ET.Element|None + self.replicated_root_memo = None # type: CDIMemo|None self._root_memos = None # type: list[CDIMemo]|None self._root_memo = None # type: CDIMemo|None self._space: Union[MemorySpace, None] = None @@ -558,7 +558,7 @@ def startElement(self, name: str, parent_cm = self._tag_stack[-1] cm = CDIMemo(tag=name, element=el, parent=parent_cm, document=self) # cm.space = self._tmp_space Commented since not replicated! - # - address and space should be set by expandedTree or the + # - address and space should be set by replicatedTree or the # node processing the CDI, accounting for replication. if name == "segment": self._tmp_space = attrib.get('space') @@ -575,7 +575,7 @@ def startElement(self, name: str, raise AttributeError( f"Node specifies {name} offset before segment origin") self._tmp_address += offset - # NOTE: ^ Sanity check only! For real address see expandedTree. + # NOTE: ^ Sanity check only! For real address see replicatedTree. self.onPushScope(cm) if len(self._tag_stack) < 1: @@ -703,7 +703,7 @@ def characters(self, content: str): "Expected str, got {}".format(type(content).__name__)) self._chunks.append(content) - def expandedTree(self) -> Tuple[CDIMemo, ET.Element]: + def replicatedTree(self) -> Tuple[CDIMemo, ET.Element]: """Build an expanded XML tree with replication and addresses. Starting from the root CDIMemo (via :meth:`getRootMemo`), this @@ -715,7 +715,7 @@ def expandedTree(self) -> Tuple[CDIMemo, ET.Element]: elements in the new tree. The original tree is left unchanged. Returns: - ET.Element: Root of the new expanded tree. + ET.Element: Root of the new replicated tree. """ root_memo = self.getRootMemo() assert root_memo is not None and root_memo.element is not None @@ -733,20 +733,20 @@ def expandedTree(self) -> Tuple[CDIMemo, ET.Element]: root_memo.element = tmp_el # restore the old copy. new_root_memo.document = self new_root_memo.element = new_root # use this not root_memo.element copy - size = self._expanded_tree_recursive(new_root_memo, new_root, + size = self._replicated_tree_recursive(new_root_memo, new_root, address=0) if size < 1: logger.warning( f"No space used by CDI after replication (size={size})") return new_root_memo, new_root - def _expanded_tree_recursive( + def _replicated_tree_recursive( self, parent: CDIMemo, parent_el: ET.Element, allow_non_standard=False, address: int = 0, space: Union[int, None] = None, ) -> int: - """Recursive helper for :meth:`expandedTree`. + """Recursive helper for :meth:`replicatedTree`. Copies the element, handles replication, sets addresses, and recurses into children. Removes ``replication`` attribute from @@ -762,7 +762,7 @@ def _expanded_tree_recursive( parent_el.text = parent.content elif parent_tag_lower in ("name", "description"): logger.warning( - f"expanded {parent_tag_lower} has no content.") + f"replicated {parent_tag_lower} has no content.") if parent_el.tail: parent.tail = parent_el.tail elif parent.tail: # new_root @@ -807,8 +807,8 @@ def _expanded_tree_recursive( # copy_child_el = child_el # copy_child_memo = child_memo # NOTE: ^ Why commented: We don't want to modify - # self.etree children (if we modify expandedTree - # result such as self.expanded_root)! + # self.etree children (if we modify replicatedTree + # result such as self.replicated_root)! # - Don't even chance it by keeping the memo # (otherwise child_memo.element would be from tree). # - Also, we always add child to parent_el below. @@ -817,7 +817,7 @@ def _expanded_tree_recursive( # for child_el in new_parent: # copy_parent_el.append(child_el) - # Remove replication from the expanded copy + # Remove replication from the replicated copy if "replication" in copy_child_el.attrib: del copy_child_el.attrib["replication"] @@ -865,7 +865,7 @@ def _expanded_tree_recursive( else: assert not size, el_error - address = self._expanded_tree_recursive( + address = self._replicated_tree_recursive( copy_child_memo, copy_child_el, address=address, space=space) if c_tag_lower == "segment": @@ -874,14 +874,14 @@ def _expanded_tree_recursive( parent.children = new_children # Same references if no replication return address - def extractCDIVarMemos(self, expanded_root=None, root_memo=None) -> List[CDIMemo]: # noqa: E501 + def extractCDIVarMemos(self, replicated_root=None, root_memo=None) -> List[CDIMemo]: # noqa: E501 # type: (ET.Element|None, CDIMemo|None) -> List[CDIMemo] """Build a flat list of CDIMemo objects for all variables. - Uses the expanded tree (replication expanded, replication + Uses the replicated tree (replication expanded, replication attribute removed, addresses set). Returns original-style memos (with .content) but with .element pointing into the - expanded tree so that modifications (setData etc.) affect + replicated tree so that modifications (setData etc.) affect the saved XML. """ # TODO: Implement ACDI vars if present (See OpenLCB @@ -889,18 +889,18 @@ def extractCDIVarMemos(self, expanded_root=None, root_memo=None) -> List[CDIMemo if not hasattr(self, "etree") or self.etree is None: logger.error("processor has no etree") return [] - if expanded_root is not None: + if replicated_root is not None: if root_memo is not None: # reserved assert isinstance(root_memo, CDIMemo) - assert isinstance(expanded_root, ET.Element) + assert isinstance(replicated_root, ET.Element) root_memo = root_memo # reserved - root_el = expanded_root + root_el = replicated_root else: - root_memo, root_el = self.expandedTree() - self.expanded_root = root_el - self.expanded_root_memo = root_memo + root_memo, root_el = self.replicatedTree() + self.replicated_root = root_el + self.replicated_root_memo = root_memo - assert isinstance(self.expanded_root_memo, CDIMemo) + assert isinstance(self.replicated_root_memo, CDIMemo) cdivar_memos: List[CDIMemo] = [] @@ -908,11 +908,11 @@ def traverse(memo: CDIMemo) -> None: tag = memo.getTag() tag_lower = tag.lower() if tag else "" if tag_lower in CLASSNAME_TYPES: - # Use the existing expanded memo (has correct .content) + # Use the existing replicated memo (has correct .content) cdivar_memos.append(memo) for child in memo.children: traverse(child) - traverse(self.expanded_root_memo) - assert root_el is self.expanded_root # concurrent modification check + traverse(self.replicated_root_memo) + assert root_el is self.replicated_root # concurrent modification check return cdivar_memos From 1a1e10cdbc31b0de5582f530ee1794f009c9929a Mon Sep 17 00:00:00 2001 From: Jacob Gustafson <7557867+Poikilos@users.noreply.github.com> Date: Wed, 20 May 2026 14:43:33 -0400 Subject: [PATCH 34/34] Remove lint. --- openlcb/cdimemo.py | 2 +- openlcb/cdivar.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/openlcb/cdimemo.py b/openlcb/cdimemo.py index ba6ad9b..84a37c4 100644 --- a/openlcb/cdimemo.py +++ b/openlcb/cdimemo.py @@ -290,7 +290,7 @@ def addChildren(self) -> None: if self.element.tail: self.tail = self.element.tail - for child_elem in list(self.element): # list() to avoid modification issues + for child_elem in list(self.element): # list() fixes concurrency issue child_memo = CDIMemo( tag=child_elem.tag, element=child_elem, diff --git a/openlcb/cdivar.py b/openlcb/cdivar.py index 26c374a..38dd44e 100644 --- a/openlcb/cdivar.py +++ b/openlcb/cdivar.py @@ -181,6 +181,7 @@ def getSerializable(self): elif self.className == "string": return self.getString() assert self.className in ("blob", "eventid", "action") + assert self.data is not None, "CDIVar data not initialized" return base64.b64encode(self.data) def getDict(self, add_name=True):