From 5c350475d35dee7ec3652d7373a9984d6ce30083 Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Tue, 30 Nov 2021 11:52:43 +1100
Subject: [PATCH 01/15] generate: device: interface improvements

- added alternate constructors `from_serial` and `from_udp` that combine instance creation and connection
- added `__enter__` and `__exit__` methods to allow using in a context manager (`initialize` on entry, `close` connection on exit)
---
 generate/templates/device.py.in | 42 +++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/generate/templates/device.py.in b/generate/templates/device.py.in
index a99496e..850e763 100644
--- a/generate/templates/device.py.in
+++ b/generate/templates/device.py.in
@@ -56,6 +56,18 @@ class PingDevice(object):
         except Exception as exception:
             raise Exception("Failed to open the given serial port: {0}".format(exception))
 
+    ##
+    # @brief Construct instance, and connect via serial
+    #
+    # @param device_name: Serial device name. E.g. /dev/ttyUSB0 or COM5
+    # @param baudrate: Connection baudrate used in the serial communication
+    #
+    @classmethod
+    def from_serial(cls, device_name: str, baudrate: int = 115200):
+        device = cls()
+        device.connect_serial(device_name, baudrate)
+        return device
+    
     ##
     # @brief Do the connection via an UDP link
     #
@@ -77,6 +89,18 @@ class PingDevice(object):
         except Exception as exception:
             raise Exception("Failed to open the given UDP port: {0}".format(exception))
 
+    ##
+    # @brief Construct instance, and connect via UDP
+    #
+    # @param host: UDP server address (IPV4) or name
+    # @param port: port used to connect with server
+    #
+    @classmethod
+    def from_udp(cls, host: str = None, port: int = 12345):
+        device = cls()
+        device.connect_udp(host, port)
+        return device
+    
     ##
     # @brief Read available data from the io device
     def read_io(self):
@@ -195,6 +219,24 @@ class PingDevice(object):
             return False
 
         return True
+    
+    ##
+    # @brief Initialize on entry to context manager.
+    #
+    def __enter__(self):
+        if not self.initialize():
+            raise Exception("{0} initialization failed.".format(self.__class__))
+        return self
+    
+    ##
+    # @brief Close connection on context manager exit (if possible).
+    #
+    def __exit__(self, *args):
+        try:
+            # may not be possible/available for some OSs or Python versions
+            self.iodev.close()
+        except Exception as exception:
+            print("Failed to close", self.__class__, "connection:", exception)
 
     ##
     # @brief Dump object into string representation.

From a811a76d1015ceb917321041e3d44477d8dc823d Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Tue, 30 Nov 2021 18:41:09 +1100
Subject: [PATCH 02/15] setup: update version to 0.2.0

Minor update: new interface functionality (alternate constructors, context managers), but still backwards compatible.
- adds Ping360 example + simulation
Need to interactive rebase to appear after the new files actually exist
---
 setup.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/setup.py b/setup.py
index 3a746cc..d22caf6 100644
--- a/setup.py
+++ b/setup.py
@@ -10,7 +10,7 @@
 """
 
 setup(name='bluerobotics-ping',
-      version='0.1.4',
+      version='0.2.0',
       python_requires='>=3.4',
       description='A python module for the Blue Robotics ping-protocol and products',
       long_description=long_description,
@@ -26,6 +26,9 @@
       ],
       scripts=[
           "examples/simplePingExample.py",
+          "examples/simplePing360Example.py",
           "tools/pingproxy.py",
-          "tools/ping1d-simulation.py"]
+          "tools/ping1d-simulation.py",
+          "tools/ping360-simulation.py",
+      ]
       )

From 94d66d0280637794ac7a1221a349a6d93f666d0c Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Tue, 30 Nov 2021 18:42:56 +1100
Subject: [PATCH 03/15] Create simplePing360Example.py

Need to overhaul so that it actually works, and is a good example.
- ideally want to use sub-parsers for serial vs udp
- needs to do something meaningful when it runs (although that could just be setting some settings and doing a full revolution or few)
---
 examples/simplePing360Example.py | 48 ++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 examples/simplePing360Example.py

diff --git a/examples/simplePing360Example.py b/examples/simplePing360Example.py
new file mode 100644
index 0000000..7f29ab9
--- /dev/null
+++ b/examples/simplePing360Example.py
@@ -0,0 +1,48 @@
+#!/usr/bin/env python
+
+#simplePing360Example.py
+from brping import Ping360
+import time
+import argparse
+
+##Parse Command line options
+############################
+
+parser = argparse.ArgumentParser(description="Ping python library example (Ping360).",
+                                 formatter_class=argparse.ArgumentDefaultsHelpFormatter)
+parser.add_argument('--device', help="Ping360 device port. E.g: /dev/ttyUSB0")
+parser.add_argument('--baudrate', type=int, default=115200, help="Ping360 device baudrate. E.g: 115200")
+parser.add_argument('--udp', help="Ping360 UDP server. E.g: 192.168.2.2:9092")
+args = parser.parse_args()
+if args.device is None and args.udp is None:
+    parser.print_help()
+    exit(1)
+
+# Make a new Ping
+device = Ping360()
+if args.device is not None:
+    device.connect_serial(args.device, args.baudrate)
+elif args.udp is not None:
+    (host, port) = args.udp.split(':')
+    device.connect_udp(host, int(port))
+
+with device:
+    line = "-" * 40
+    print(line)
+    print("Starting Ping360...")
+    print("Press CTRL+C to exit")
+    print(line)
+    
+    input("Press Enter to continue...")
+
+    # Read and print 
+    while "user hasn't quit":
+        
+# Read and print distance measurements with confidence
+while True:
+    data = myPing.get_distance()
+    if data:
+        print("Distance: %s\tConfidence: %s%%" % (data["distance"], data["confidence"]))
+    else:
+        print("Failed to get distance data")
+    time.sleep(0.1)

From b59625bdd1a4dbfbc89554881ee9d439091c35df Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 14:50:29 +1100
Subject: [PATCH 04/15] generate: device: add version properties

---
 generate/templates/device.py.in | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/generate/templates/device.py.in b/generate/templates/device.py.in
index 850e763..57be964 100644
--- a/generate/templates/device.py.in
+++ b/generate/templates/device.py.in
@@ -277,6 +277,31 @@ class PingDevice(object):
         })
         return data
 
+{# add properties for version numbers, for easier comparison #}
+{% set ns = namespace(prop = msg if "version" in msg) %}
+{%- if not ns.prop %}
+    {%- for field in messages["get"][msg].payload %}
+        {%- if not ns.prop and "version" in field.name %}
+            {%- set ns.prop = field.name[:field.name.index("version")+7] -%}
+        {% endif -%}
+    {% endfor -%}
+{% endif -%}
+{% if ns.prop %}
+    @property
+    def {{ns.prop}}(self):
+        try:
+            return (
+{% for field in messages["get"][msg].payload %}
+{% if "version" in field.name %}
+                self._{{field.name}},
+{% endif %}
+{% endfor %}
+            )
+        except AttributeError:
+            self.get_{{msg}}()
+            return self.{{ns.prop}}
+{% endif %}
+
 {% endfor %}
 
 if __name__ == "__main__":

From 23d55f5c6f6f4fb2e5a92136f1d7c744849aa9ce Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 14:54:10 +1100
Subject: [PATCH 05/15] generate: ping360: auto only if supported

---
 generate/templates/ping360.py.in | 39 +++++++++++++++++---------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/generate/templates/ping360.py.in b/generate/templates/ping360.py.in
index f946093..764c7b4 100644
--- a/generate/templates/ping360.py.in
+++ b/generate/templates/ping360.py.in
@@ -163,29 +163,32 @@ if __name__ == "__main__":
 
     print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
 
-    # turn on auto-scan with 1 grad steps
-    p.control_auto_transmit(0,399,1,0)
+    if p.firmware_version > (3, 1, 1):
+        # turn on auto-scan with 1 grad steps
+        p.control_auto_transmit(0,399,1,0)
 
-    tstart_s = time.time()
-    # wait for 400 device_data messages to arrive
-    for x in range(400):
-        p.wait_message([definitions.PING360_DEVICE_DATA])
-    tend_s = time.time()
+        tstart_s = time.time()
+        # wait for 400 device_data messages to arrive
+        for x in range(400):
+            p.wait_message([definitions.PING360_DEVICE_DATA])
+            tend_s = time.time()
 
-    print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
+        print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
 
-    # stop the auto-transmit process
-    p.control_motor_off()
+        # stop the auto-transmit process
+        p.control_motor_off()
 
-    # turn on auto-transmit with 10 grad steps
-    p.control_auto_transmit(0,399,10,0)
+        # turn on auto-transmit with 10 grad steps
+        p.control_auto_transmit(0,399,10,0)
 
-    tstart_s = time.time()
-    # wait for 40 device_data messages to arrive (40 * 10grad steps = 400 grads)
-    for x in range(40):
-        p.wait_message([definitions.PING360_DEVICE_DATA])
-    tend_s = time.time()
+        tstart_s = time.time()
+        # wait for 40 device_data messages to arrive (40 * 10grad steps = 400 grads)
+        for x in range(40):
+            p.wait_message([definitions.PING360_DEVICE_DATA])
+            tend_s = time.time()
 
-    print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
+        print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
+    else:
+        print("firmware {} does not support auto commands".format(p.firmware_version))
 
     p.control_reset(0, 0)

From 03fd6f852f91e9d5cf06164478963e3a63ece35a Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 14:56:06 +1100
Subject: [PATCH 06/15] generate: ping360: improve initialize - simplify to use
 boolean operator short-circuiting - use super instead of explicit parent
 class name

---
 generate/templates/ping360.py.in | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/generate/templates/ping360.py.in b/generate/templates/ping360.py.in
index 764c7b4..31044d1 100644
--- a/generate/templates/ping360.py.in
+++ b/generate/templates/ping360.py.in
@@ -15,11 +15,7 @@ import time
 
 class Ping360(PingDevice):
     def initialize(self):
-        if not PingDevice.initialize(self):
-            return False
-        if (self.readDeviceInformation() is None):
-            return False
-        return True
+        return super().initialize() and self.readDeviceInformation() is not None
 
 {% for msg in messages["get"]|sort %}
     ##

From 9f25f00b22e393b2fc4632cdc49cfa04d305772e Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 14:59:31 +1100
Subject: [PATCH 07/15] generate: ping360: simplify imports

---
 generate/templates/ping360.py.in | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/generate/templates/ping360.py.in b/generate/templates/ping360.py.in
index 31044d1..770314d 100644
--- a/generate/templates/ping360.py.in
+++ b/generate/templates/ping360.py.in
@@ -8,9 +8,7 @@
 # DO NOT EDIT
 # ~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!~!
 
-from brping import definitions
-from brping import PingDevice
-from brping import pingmessage
+from brping import definitions, PingDevice, PingMessage
 import time
 
 class Ping360(PingDevice):
@@ -51,7 +49,7 @@ class Ping360(PingDevice):
     #
     # @return If verify is False, True on successful communication with the device. If verify is False, True if the new device parameters are verified to have been written correctly. False otherwise (failure to read values back or on verification failure)
     def {{msg}}(self{% for field in messages["set"][msg].payload %}, {{field.name}}{% endfor %}, verify=True):
-        m = pingmessage.PingMessage(definitions.PING360_{{msg|upper}})
+        m = PingMessage(definitions.PING360_{{msg|upper}})
 {% for field in messages["set"][msg].payload %}
         m.{{field.name}} = {{field.name}}
 {% endfor %}
@@ -75,7 +73,7 @@ class Ping360(PingDevice):
 
 {% for msg in messages["control"]|sort %}
     def control_{{msg}}(self{% for field in messages["control"][msg].payload %}, {{field.name}}{% endfor %}):
-        m = pingmessage.PingMessage(definitions.PING360_{{msg|upper}})
+        m = PingMessage(definitions.PING360_{{msg|upper}})
 {% for field in messages["control"][msg].payload %}
         m.{{field.name}} = {{field.name}}
 {% endfor %}

From 58f8fc1a6d6f0f1fe1806841fef72a97f3b39119 Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 18:49:39 +1100
Subject: [PATCH 08/15] generate: ping360: don't reset by default Resetting can
 cause issues with using the sonar again until it's power cycled Particularly
 relevant given we currently don't have discovery messages implemented.

---
 generate/templates/ping360.py.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/generate/templates/ping360.py.in b/generate/templates/ping360.py.in
index 770314d..4b7d701 100644
--- a/generate/templates/ping360.py.in
+++ b/generate/templates/ping360.py.in
@@ -185,4 +185,4 @@ if __name__ == "__main__":
     else:
         print("firmware {} does not support auto commands".format(p.firmware_version))
 
-    p.control_reset(0, 0)
+    #p.control_reset(0, 0)

From 949cd02f28db5ab2924d352ec2e884984e7fd9ee Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 18:51:51 +1100
Subject: [PATCH 09/15] definitions: change to using a Payload NamedTuple Using
 a dedicated datastructure improves robustness (you can't forget a field, and
 all payloads have the same fields). A NamedTuple adds typing (requires Python
 >= 3.6) and reduces memory usage and attribute access time.

---
 brping/pingmessage.py                         | 28 +++++++--------
 generate/generate-python.py                   | 34 +++++++++++++------
 generate/templates/device.py.in               |  2 +-
 .../templates/pingmessage-definitions.py.in   | 12 +++----
 4 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/brping/pingmessage.py b/brping/pingmessage.py
index 3a31851..8804ceb 100644
--- a/brping/pingmessage.py
+++ b/brping/pingmessage.py
@@ -98,10 +98,10 @@ def __init__(self, msg_id=0, msg_data=None):
 
             try:
                 ## The name of this message
-                self.name = payload_dict[self.message_id]["name"]
+                self.name = payload_dict[self.message_id].name
 
                 ## The field names of this message
-                self.payload_field_names = payload_dict[self.message_id]["field_names"]
+                self.payload_field_names = payload_dict[self.message_id].field_names
 
                 # initialize payload field members
                 for attr in self.payload_field_names:
@@ -133,7 +133,7 @@ def pack_msg_data(self):
         msg_format = PingMessage.endianess + PingMessage.header_format + self.get_payload_format()
 
         # Prepare complete list of field names (header + payload)
-        attrs = PingMessage.header_field_names + payload_dict[self.message_id]["field_names"]
+        attrs = PingMessage.header_field_names + payload_dict[self.message_id].field_names
 
         # Prepare iterable ordered list of values to pack
         values = []
@@ -165,13 +165,13 @@ def unpack_msg_data(self, msg_data):
 
         ## The name of this message
         try:
-            self.name = payload_dict[self.message_id]["name"]
+            self.name = payload_dict[self.message_id].name
         except KeyError:
             print("Unknown message: ", self.message_id)
             return False
 
         ## The field names of this message
-        self.payload_field_names = payload_dict[self.message_id]["field_names"]
+        self.payload_field_names = payload_dict[self.message_id].field_names
 
         if self.payload_length > 0:
             ## The struct formatting string for the message payload
@@ -217,22 +217,22 @@ def verify_checksum(self):
     def update_payload_length(self):
         if self.message_id in variable_msgs or self.message_id in asciiMsgs:
             # The last field self.payload_field_names[-1] is always the single dynamic-length field
-            self.payload_length = payload_dict[self.message_id]["payload_length"] + len(getattr(self, self.payload_field_names[-1]))
+            self.payload_length = payload_dict[self.message_id].payload_length + len(getattr(self, self.payload_field_names[-1]))
         else:
-            self.payload_length = payload_dict[self.message_id]["payload_length"]
+            self.payload_length = payload_dict[self.message_id].payload_length
 
     ## Get the python struct formatting string for the message payload
     # @return the payload struct format string
     def get_payload_format(self):
         # messages with variable length fields
         if self.message_id in variable_msgs or self.message_id in asciiMsgs:
-            var_length = self.payload_length - payload_dict[self.message_id]["payload_length"]  # Subtract static length portion from payload length
+            var_length = self.payload_length - payload_dict[self.message_id].payload_length  # Subtract static length portion from payload length
             if var_length <= 0:
-                return payload_dict[self.message_id]["format"]  # variable data portion is empty
+                return payload_dict[self.message_id].format  # variable data portion is empty
 
-            return payload_dict[self.message_id]["format"] + str(var_length) + "s"
+            return payload_dict[self.message_id].format + str(var_length) + "s"
         else: # messages with a static (constant) length
-            return payload_dict[self.message_id]["format"]
+            return payload_dict[self.message_id].format
 
     ## Dump object into string representation
     # @return string representation of the object
@@ -250,17 +250,17 @@ def __repr__(self):
             if self.message_id in variable_msgs:
 
                 # static fields are handled as usual
-                for attr in payload_dict[self.message_id]["field_names"][:-1]:
+                for attr in payload_dict[self.message_id].field_names[:-1]:
                     payload_string += "\n  - " + attr + ": " + str(getattr(self, attr))
 
                 # the variable length field is always the last field
-                attr = payload_dict[self.message_id]["field_names"][-1:][0]
+                attr = payload_dict[self.message_id].field_names[-1:][0]
 
                 # format this field as a list of hex values (rather than a string if we did not perform this handling)
                 payload_string += "\n  - " + attr + ": " + str([hex(item) for item in getattr(self, attr)])
 
             else:  # handling of static length messages and text messages
-                for attr in payload_dict[self.message_id]["field_names"]:
+                for attr in payload_dict[self.message_id].field_names:
                     payload_string += "\n  - " + attr + ": " + str(getattr(self, attr))
 
         representation = (
diff --git a/generate/generate-python.py b/generate/generate-python.py
index d465e42..edfb113 100755
--- a/generate/generate-python.py
+++ b/generate/generate-python.py
@@ -38,24 +38,38 @@
                 "i32": "i",
                 "char": "s"}
 
+payload_setup = '''\
+from typing import NamedTuple, Tuple
+
+class Payload(NamedTuple):
+    name: str
+    format: str
+    field_names: Tuple[str]
+    payload_length: int
+
+'''
+
 f = open("%s/definitions.py" % args.output_directory, "w")
 
+f.write(payload_setup)
+
 for definition in definitions:
     definitionFile = "%s/%s.json" % (definitionPath, definition)
     f.write(g.generate(definitionFile, templateFile, {"structToken": struct_token, "base": definition}))
 
 #allString = "payload_dict_all = {}\n"
 # add PINGMESSAGE_UNDEFINED for legacy request support
-allString = '\
-PINGMESSAGE_UNDEFINED = 0\n\
-payload_dict_all = {\n\
-    PINGMESSAGE_UNDEFINED: {\n\
-        "name": "undefined",\n\
-        "format": "",\n\
-        "field_names": (),\n\
-        "payload_length": 0\n\
-    },\n\
-}\n'
+allString = '''\
+PINGMESSAGE_UNDEFINED = 0
+payload_dict_all = {
+    PINGMESSAGE_UNDEFINED: Payload(
+        name = "undefined",
+        format = "",
+        field_names = (),
+        payload_length = 0
+    ),
+}
+'''
 
 f.write(allString)
 
diff --git a/generate/templates/device.py.in b/generate/templates/device.py.in
index 57be964..d809e64 100644
--- a/generate/templates/device.py.in
+++ b/generate/templates/device.py.in
@@ -209,7 +209,7 @@ class PingDevice(object):
 
         if msg.message_id in pingmessage.payload_dict:
             try:
-                for attr in pingmessage.payload_dict[msg.message_id]["field_names"]:
+                for attr in pingmessage.payload_dict[msg.message_id].field_names:
                     setattr(self, "_" + attr, getattr(msg, attr))
             except AttributeError as e:
                 print("attribute error while handling msg %d (%s): %s" % (msg.message_id, msg.name, msg.msg_data))
diff --git a/generate/templates/pingmessage-definitions.py.in b/generate/templates/pingmessage-definitions.py.in
index eb5d6b7..5f1ea48 100644
--- a/generate/templates/pingmessage-definitions.py.in
+++ b/generate/templates/pingmessage-definitions.py.in
@@ -12,9 +12,9 @@ payload_dict_{{base}} = {
 {% for msg_type in messages %}
 {% for msg in messages[msg_type] %}
 {% set m = messages[msg_type][msg] %}
-    {{base|upper}}_{{msg|upper}}: {
-        "name": "{{msg}}",
-        "format": "
+    {{base|upper}}_{{msg|upper}}: Payload(
+        name = "{{msg}}",
+        format = "
 {%- for field in m.payload %}
 {% if generator.is_vector(field) %}
 {% if field.vector.sizetype %}
@@ -24,7 +24,7 @@ payload_dict_{{base}} = {
 {{structToken[field.type]}}
 {%- endif %}
 {% endfor %}{# for each field #}",
-        "field_names": (
+        field_names = (
 {% for field in m.payload %}
 {% if generator.is_vector(field) %}
 {% if field.vector.sizetype %}
@@ -34,8 +34,8 @@ payload_dict_{{base}} = {
              "{{field.name}}",
 {% endfor %}{# for each field #}
             ),
-        "payload_length": {{generator.calc_payload(m.payload)}}
-    },
+        payload_length = {{generator.calc_payload(m.payload)}}
+    ),
 
 {% endfor %}
 {% endfor %}

From 838cf9830c8ab1dca97c9b1b1f5d1de1b5aa7f60 Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 20:24:46 +1100
Subject: [PATCH 10/15] setup.py: require Python >= 3.6 for typing.NamedTuple

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index d22caf6..b7ae896 100644
--- a/setup.py
+++ b/setup.py
@@ -11,7 +11,7 @@
 
 setup(name='bluerobotics-ping',
       version='0.2.0',
-      python_requires='>=3.4',
+      python_requires='>=3.6',
       description='A python module for the Blue Robotics ping-protocol and products',
       long_description=long_description,
       long_description_content_type='text/markdown',

From 6334ea5fb9f1f6949289a36029097ccf67237eba Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 21:48:23 +1100
Subject: [PATCH 11/15] pingmessage: switch to message enums

---
 brping/pingmessage.py                         | 10 +++++-----
 generate/generate-python.py                   |  9 +++++++--
 generate/templates/device.py.in               |  9 +++++----
 generate/templates/ping1d.py.in               | 11 +++++-----
 generate/templates/ping360.py.in              | 20 ++++++++++---------
 .../templates/pingmessage-definitions.py.in   |  5 +++--
 6 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/brping/pingmessage.py b/brping/pingmessage.py
index 8804ceb..2a6ee58 100644
--- a/brping/pingmessage.py
+++ b/brping/pingmessage.py
@@ -6,8 +6,8 @@
 import struct
 from brping import definitions
 payload_dict = definitions.payload_dict_all
-asciiMsgs = [definitions.COMMON_NACK, definitions.COMMON_ASCII_TEXT]
-variable_msgs = [definitions.PING1D_PROFILE, definitions.PING360_DEVICE_DATA, ]
+asciiMsgs = [definitions.CommonMessage.NACK, definitions.CommonMessage.ASCII_TEXT]
+variable_msgs = [definitions.Ping1dMessage.PROFILE, definitions.Ping360Message.DEVICE_DATA, ]
 
 
 class PingMessage(object):
@@ -53,7 +53,7 @@ class PingMessage(object):
     #
     # @par Ex set:
     # @code
-    # m = PingMessage(PING1D_SET_RANGE)
+    # m = PingMessage(Ping1dMessage.SET_RANGE)
     # m.start_mm = 1000
     # m.length_mm = 2000
     # m.update_checksum()
@@ -63,7 +63,7 @@ class PingMessage(object):
     # @par Ex receive:
     # @code
     # m = PingMessage(rxByteArray)
-    # if m.message_id == PING1D_RANGE
+    # if m.message_id == Ping1dMessage.RANGE
     #     start_mm = m.start_mm
     #     length_mm = m.length_mm
     # @endcode
@@ -419,7 +419,7 @@ def parse_byte(self, msg_byte):
         0x52,
         4,
         0,
-        definitions.COMMON_PROTOCOL_VERSION,
+        definitions.CommonMessage.PROTOCOL_VERSION,
         0,
         77,
         211,
diff --git a/generate/generate-python.py b/generate/generate-python.py
index edfb113..2dbc67f 100755
--- a/generate/generate-python.py
+++ b/generate/generate-python.py
@@ -40,6 +40,7 @@
 
 payload_setup = '''\
 from typing import NamedTuple, Tuple
+from enum import IntEnum
 
 class Payload(NamedTuple):
     name: str
@@ -47,6 +48,8 @@ class Payload(NamedTuple):
     field_names: Tuple[str]
     payload_length: int
 
+class MessageEnum(IntEnum): pass
+
 '''
 
 f = open("%s/definitions.py" % args.output_directory, "w")
@@ -60,9 +63,11 @@ class Payload(NamedTuple):
 #allString = "payload_dict_all = {}\n"
 # add PINGMESSAGE_UNDEFINED for legacy request support
 allString = '''\
-PINGMESSAGE_UNDEFINED = 0
+class UndefinedMessage(MessageEnum):
+    UNDEFINED = 0
+
 payload_dict_all = {
-    PINGMESSAGE_UNDEFINED: Payload(
+    UndefinedMessage.UNDEFINED: Payload(
         name = "undefined",
         format = "",
         field_names = (),
diff --git a/generate/templates/device.py.in b/generate/templates/device.py.in
index d809e64..8f8f647 100644
--- a/generate/templates/device.py.in
+++ b/generate/templates/device.py.in
@@ -16,6 +16,7 @@ import socket
 import time
 
 class PingDevice(object):
+    messages = definitions.CommonMessage
 {% for field in all_fields|sort %}
     _{{field}} = None
 {% endfor%}
@@ -156,7 +157,7 @@ class PingDevice(object):
     #
     # @return True if the device replies with expected data, False otherwise
     def initialize(self):
-        return self.request(definitions.COMMON_PROTOCOL_VERSION) is not None
+        return self.request(definitions.CommonMessage.PROTOCOL_VERSION) is not None
 
     ##
     # @brief Request the given message ID
@@ -169,13 +170,13 @@ class PingDevice(object):
     #
     # @todo handle nack to exit without blocking
     def request(self, m_id, timeout=0.5):
-        msg = pingmessage.PingMessage(definitions.COMMON_GENERAL_REQUEST)
+        msg = pingmessage.PingMessage(definitions.CommonMessage.GENERAL_REQUEST)
         msg.requested_id = m_id
         msg.pack_msg_data()
         self.write(msg.msg_data)
 
         # uncomment to return nacks in addition to m_id
-        # return self.wait_message([m_id, definitions.COMMON_NACK], timeout)
+        # return self.wait_message([m_id, definitions.CommonMessage.NACK], timeout)
 
         return self.wait_message([m_id], timeout)
 
@@ -268,7 +269,7 @@ class PingDevice(object):
     # {{field.name}}: {% if field.units %}Units: {{field.units}}; {% endif %}{{field.description}}\n
 {% endfor%}
     def get_{{msg}}(self):
-        if self.request(definitions.COMMON_{{msg|upper}}) is None:
+        if self.request(definitions.CommonMessage.{{msg|upper}}) is None:
             return None
         data = ({
 {% for field in messages["get"][msg].payload %}
diff --git a/generate/templates/ping1d.py.in b/generate/templates/ping1d.py.in
index 810ef2c..49a6ae2 100644
--- a/generate/templates/ping1d.py.in
+++ b/generate/templates/ping1d.py.in
@@ -13,6 +13,7 @@ from brping import PingDevice
 from brping import pingmessage
 
 class Ping1D(PingDevice):
+    messages = definitions.Ping1dMessage
 
     def legacyRequest(self, m_id, timeout=0.5):
         msg = pingmessage.PingMessage()
@@ -23,14 +24,14 @@ class Ping1D(PingDevice):
         self.write(msg.msg_data)
 
         # uncomment to return nacks in addition to m_id
-        # return self.wait_message([m_id, definitions.COMMON_NACK], timeout)
+        # return self.wait_message([m_id, super().messages.NACK], timeout)
 
         return self.wait_message([m_id], timeout)
 
     def initialize(self):
         if not PingDevice.initialize(self):
             return False
-        if self.legacyRequest(definitions.PING1D_GENERAL_INFO) is None:
+        if self.legacyRequest(self.messages.GENERAL_INFO) is None:
             return False
         return True
 
@@ -45,7 +46,7 @@ class Ping1D(PingDevice):
     # {{field.name}}: {% if field.units %}Units: {{field.units}}; {% endif %}{{field.description}}\n
 {% endfor%}
     def get_{{msg}}(self):
-        if self.legacyRequest(definitions.PING1D_{{msg|upper}}) is None:
+        if self.legacyRequest(self.messages.{{msg|upper}}) is None:
             return None
         data = ({
 {% for field in messages["get"][msg].payload %}
@@ -68,13 +69,13 @@ class Ping1D(PingDevice):
     #
     # @return If verify is False, True on successful communication with the device. If verify is False, True if the new device parameters are verified to have been written correctly. False otherwise (failure to read values back or on verification failure)
     def {{msg}}(self{% for field in messages["set"][msg].payload %}, {{field.name}}{% endfor %}, verify=True):
-        m = pingmessage.PingMessage(definitions.PING1D_{{msg|upper}})
+        m = pingmessage.PingMessage(self.messages.{{msg|upper}})
 {% for field in messages["set"][msg].payload %}
         m.{{field.name}} = {{field.name}}
 {% endfor %}
         m.pack_msg_data()
         self.write(m.msg_data)
-        if self.legacyRequest(definitions.PING1D_{{msg|replace("set_", "")|upper}}) is None:
+        if self.legacyRequest(self.messages.{{msg|replace("set_", "")|upper}}) is None:
             return False
         # Read back the data and check that changes have been applied
         if (verify
diff --git a/generate/templates/ping360.py.in b/generate/templates/ping360.py.in
index 4b7d701..bf916a4 100644
--- a/generate/templates/ping360.py.in
+++ b/generate/templates/ping360.py.in
@@ -12,6 +12,8 @@ from brping import definitions, PingDevice, PingMessage
 import time
 
 class Ping360(PingDevice):
+    messages = definitions.Ping360Message
+
     def initialize(self):
         return super().initialize() and self.readDeviceInformation() is not None
 
@@ -26,7 +28,7 @@ class Ping360(PingDevice):
     # {{field.name}}: {% if field.units %}Units: {{field.units}}; {% endif %}{{field.description}}\n
 {% endfor%}
     def get_{{msg}}(self):
-        if self.request(definitions.PING360_{{msg|upper}}) is None:
+        if self.request(self.messages.{{msg|upper}}) is None:
             return None
         data = ({
 {% for field in messages["get"][msg].payload %}
@@ -49,13 +51,13 @@ class Ping360(PingDevice):
     #
     # @return If verify is False, True on successful communication with the device. If verify is False, True if the new device parameters are verified to have been written correctly. False otherwise (failure to read values back or on verification failure)
     def {{msg}}(self{% for field in messages["set"][msg].payload %}, {{field.name}}{% endfor %}, verify=True):
-        m = PingMessage(definitions.PING360_{{msg|upper}})
+        m = PingMessage(self.messages.{{msg|upper}})
 {% for field in messages["set"][msg].payload %}
         m.{{field.name}} = {{field.name}}
 {% endfor %}
         m.pack_msg_data()
         self.write(m.msg_data)
-        if self.request(definitions.PING360_{{msg|replace("set_", "")|upper}}) is None:
+        if self.request(self.messages.{{msg|replace("set_", "")|upper}}) is None:
             return False
         # Read back the data and check that changes have been applied
         if (verify
@@ -73,7 +75,7 @@ class Ping360(PingDevice):
 
 {% for msg in messages["control"]|sort %}
     def control_{{msg}}(self{% for field in messages["control"][msg].payload %}, {{field.name}}{% endfor %}):
-        m = PingMessage(definitions.PING360_{{msg|upper}})
+        m = PingMessage(self.messages.{{msg|upper}})
 {% for field in messages["control"][msg].payload %}
         m.{{field.name}} = {{field.name}}
 {% endfor %}
@@ -98,13 +100,13 @@ class Ping360(PingDevice):
             0,
             0
         )
-        return self.wait_message([definitions.PING360_DEVICE_DATA, definitions.COMMON_NACK], 4.0)
+        return self.wait_message([self.messages.DEVICE_DATA, super().messages.NACK], 4.0)
 
 {% endif %}
 {% endfor %}
 
     def readDeviceInformation(self):
-        return self.request(definitions.PING360_DEVICE_DATA)
+        return self.request(self.messages.DEVICE_DATA)
 
     def transmitAngle(self, angle):
         self.control_transducer(
@@ -118,7 +120,7 @@ class Ping360(PingDevice):
             1,
             0
         )
-        return self.wait_message([definitions.PING360_DEVICE_DATA, definitions.COMMON_NACK], 4.0)
+        return self.wait_message([self.messages.DEVICE_DATA, super().messages.NACK], 4.0)
     
     def transmit(self):
         return self.transmitAngle(self._angle)
@@ -164,7 +166,7 @@ if __name__ == "__main__":
         tstart_s = time.time()
         # wait for 400 device_data messages to arrive
         for x in range(400):
-            p.wait_message([definitions.PING360_DEVICE_DATA])
+            p.wait_message([definitions.Ping360Message.AUTO_DEVICE_DATA])
             tend_s = time.time()
 
         print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
@@ -178,7 +180,7 @@ if __name__ == "__main__":
         tstart_s = time.time()
         # wait for 40 device_data messages to arrive (40 * 10grad steps = 400 grads)
         for x in range(40):
-            p.wait_message([definitions.PING360_DEVICE_DATA])
+            p.wait_message([definitions.Ping360Message.AUTO_DEVICE_DATA])
             tend_s = time.time()
 
         print("full scan in %dms, %dHz" % (1000*(tend_s - tstart_s), 400/(tend_s - tstart_s)))
diff --git a/generate/templates/pingmessage-definitions.py.in b/generate/templates/pingmessage-definitions.py.in
index 5f1ea48..5c62a32 100644
--- a/generate/templates/pingmessage-definitions.py.in
+++ b/generate/templates/pingmessage-definitions.py.in
@@ -1,7 +1,8 @@
+class {{base|title}}Message(MessageEnum):
 {% for msg_type in messages %}
 {% for msg in messages[msg_type] %}
 {% set m = messages[msg_type][msg] %}
-{{base|upper}}_{{msg|upper}} = {{m.id}}
+    {{msg|upper}} = {{m.id}}
 {% endfor %}
 {% endfor %}
 
@@ -12,7 +13,7 @@ payload_dict_{{base}} = {
 {% for msg_type in messages %}
 {% for msg in messages[msg_type] %}
 {% set m = messages[msg_type][msg] %}
-    {{base|upper}}_{{msg|upper}}: Payload(
+    {{base|title}}Message.{{msg|upper}}: Payload(
         name = "{{msg}}",
         format = "
 {%- for field in m.payload %}

From 73ac4f3b9d53e517fff8aa1d2b87cb8d8a80cf52 Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Sun, 20 Mar 2022 21:54:30 +1100
Subject: [PATCH 12/15] pingmessage: PingMessage -> self internally

---
 brping/pingmessage.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/brping/pingmessage.py b/brping/pingmessage.py
index 2a6ee58..1d0d1cc 100644
--- a/brping/pingmessage.py
+++ b/brping/pingmessage.py
@@ -130,10 +130,10 @@ def pack_msg_data(self):
         self.update_payload_length()
 
         # Prepare struct packing format string
-        msg_format = PingMessage.endianess + PingMessage.header_format + self.get_payload_format()
+        msg_format = self.endianess + self.header_format + self.get_payload_format()
 
         # Prepare complete list of field names (header + payload)
-        attrs = PingMessage.header_field_names + payload_dict[self.message_id].field_names
+        attrs = self.header_field_names + payload_dict[self.message_id].field_names
 
         # Prepare iterable ordered list of values to pack
         values = []
@@ -148,7 +148,7 @@ def pack_msg_data(self):
         self.msg_data = bytearray(struct.pack(msg_format, *values))
 
         # Update and append checksum
-        self.msg_data += bytearray(struct.pack(PingMessage.endianess + PingMessage.checksum_format, self.update_checksum()))
+        self.msg_data += bytearray(struct.pack(self.endianess + self.checksum_format, self.update_checksum()))
 
         return self.msg_data
 
@@ -158,9 +158,9 @@ def unpack_msg_data(self, msg_data):
         self.msg_data = msg_data
 
         # Extract header
-        header = struct.unpack(PingMessage.endianess + PingMessage.header_format, self.msg_data[0:PingMessage.headerLength])
+        header = struct.unpack(self.endianess + self.header_format, self.msg_data[0:self.headerLength])
 
-        for i, attr in enumerate(PingMessage.header_field_names):
+        for i, attr in enumerate(self.header_field_names):
             setattr(self, attr, header[i])
 
         ## The name of this message
@@ -179,11 +179,11 @@ def unpack_msg_data(self, msg_data):
 
             # Extract payload
             try:
-                payload = struct.unpack(PingMessage.endianess + self.payload_format, self.msg_data[PingMessage.headerLength:PingMessage.headerLength + self.payload_length])
+                payload = struct.unpack(self.endianess + self.payload_format, self.msg_data[self.headerLength:self.headerLength + self.payload_length])
             except Exception as e:
                 print("error unpacking payload: %s" % e)
                 print("msg_data: %s, header: %s" % (msg_data, header))
-                print("format: %s, buf: %s" % (PingMessage.endianess + self.payload_format, self.msg_data[PingMessage.headerLength:PingMessage.headerLength + self.payload_length]))
+                print("format: %s, buf: %s" % (self.endianess + self.payload_format, self.msg_data[self.headerLength:self.headerLength + self.payload_length]))
                 print(self.payload_format)
             else:  # only use payload if didn't raise exception
                 for i, attr in enumerate(self.payload_field_names):
@@ -196,12 +196,12 @@ def unpack_msg_data(self, msg_data):
                             pass
 
         # Extract checksum
-        self.checksum = struct.unpack(PingMessage.endianess + PingMessage.checksum_format, self.msg_data[PingMessage.headerLength + self.payload_length: PingMessage.headerLength + self.payload_length + PingMessage.checksumLength])[0]
+        self.checksum = struct.unpack(self.endianess + self.checksum_format, self.msg_data[self.headerLength + self.payload_length: self.headerLength + self.payload_length + self.checksumLength])[0]
         return True
 
     ## Calculate the checksum from the internal bytearray self.msg_data
     def calculate_checksum(self):
-        return sum(self.msg_data[0:PingMessage.headerLength + self.payload_length]) & 0xffff
+        return sum(self.msg_data[0:self.headerLength + self.payload_length]) & 0xffff
 
     ## Update the object checksum value
     # @return the object checksum value
@@ -238,7 +238,7 @@ def get_payload_format(self):
     # @return string representation of the object
     def __repr__(self):
         header_string = "Header:"
-        for attr in PingMessage.header_field_names:
+        for attr in self.header_field_names:
             header_string += " " + attr + ": " + str(getattr(self, attr))
 
         if self.payload_length == 0:  # this is a hack/guard for empty body requests

From b9e02139e17d8920cbde01fb82726adb9b223091 Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Fri, 15 Apr 2022 15:01:19 +1000
Subject: [PATCH 13/15] pingmessage: improve PingMessage instance creation  -
 separated data-buffer construction into its own method (`from_buffer`)  -
 improved primary constructor to allow specifying message data directly

---
 brping/pingmessage.py | 63 ++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/brping/pingmessage.py b/brping/pingmessage.py
index 1d0d1cc..d5614d9 100644
--- a/brping/pingmessage.py
+++ b/brping/pingmessage.py
@@ -67,7 +67,7 @@ class PingMessage(object):
     #     start_mm = m.start_mm
     #     length_mm = m.length_mm
     # @endcode
-    def __init__(self, msg_id=0, msg_data=None):
+    def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_fields):
         ## The message id
         self.message_id = msg_id
 
@@ -75,9 +75,9 @@ def __init__(self, msg_id=0, msg_data=None):
         self.request_id = None
 
         ## The message destination
-        self.dst_device_id = 0
+        self.dst_device_id = dst_device_id
         ## The message source
-        self.src_device_id = 0
+        self.src_device_id = src_device_id
         ## The message checksum
         self.checksum = 0
 
@@ -85,42 +85,45 @@ def __init__(self, msg_id=0, msg_data=None):
         # update with pack_msg_data()
         self.msg_data = None
 
-        # Constructor 1: make a pingmessage object from a binary data buffer
-        # (for receiving + unpacking)
-        if msg_data is not None:
-            if not self.unpack_msg_data(msg_data):
-                # Attempted to create an unknown message
-                return
         # Constructor 2: make a pingmessage object cooresponding to a message
         # id, with field members ready to access and populate
         # (for packing + transmitting)
-        else:
+        try:
+            ## The name of this message
+            self.name = payload_dict[self.message_id].name
 
-            try:
-                ## The name of this message
-                self.name = payload_dict[self.message_id].name
+            ## The field names of this message
+            self.payload_field_names = payload_dict[self.message_id].field_names
 
-                ## The field names of this message
-                self.payload_field_names = payload_dict[self.message_id].field_names
+            # initialize payload field members
+            for attr in self.payload_field_names:
+                setattr(self, attr, payload_fields.get(attr, 0))
 
-                # initialize payload field members
-                for attr in self.payload_field_names:
-                    setattr(self, attr, 0)
+            # initialize vector field if present in message
+            if self.message_id in variable_msgs:
+                last_field = self.payload_field_names[-1]
+                # only set if not already set by user
+                if getattr(self, last_field) == 0:
+                    setattr(self, last_field, bytearray())
 
-                # initialize vector fields
-                if self.message_id in variable_msgs:
-                    setattr(self, self.payload_field_names[-1], bytearray())
+            ## Number of bytes in the message payload
+            self.update_payload_length()
 
-                ## Number of bytes in the message payload
-                self.update_payload_length()
+            ## The struct formatting string for the message payload
+            self.payload_format = self.get_payload_format()
 
-                ## The struct formatting string for the message payload
-                self.payload_format = self.get_payload_format()
+        except KeyError as e:
+            message_id = self.message_id
+            raise Exception(f"{message_id = } not recognized\n{msg_data = }") from e
 
-            # TODO handle better here, and catch Constructor 1 also
-            except KeyError as e:
-                print("message id not recognized: %d" % self.message_id, msg_data)
-                raise e
+    @classmethod
+    def from_buffer(cls, msg_data):
+        """ Alternate constructor - initialise from a binary data buffer. """
+        msg = cls()
+        if not msg.unpack_msg_data(msg_data):
+            # Attempted to create an unknown message
+            return
+        return msg
 
     ## Pack object attributes into self.msg_data (bytearray)
     # @return self.msg_data
@@ -369,7 +372,7 @@ def wait_checksum_h(self, msg_byte):
         self.message_id = 0
 
         self.buf.append(msg_byte)
-        self.rx_msg = PingMessage(msg_data=self.buf)
+        self.rx_msg = PingMessage.from_buffer(self.buf)
 
         if self.rx_msg.verify_checksum():
             self.parsed += 1

From 37fb6c6c758805d7d29814c86307269bea77c169 Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Fri, 15 Apr 2022 16:06:25 +1000
Subject: [PATCH 14/15] fixup

---
 brping/pingmessage.py | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/brping/pingmessage.py b/brping/pingmessage.py
index d5614d9..d4ecf33 100644
--- a/brping/pingmessage.py
+++ b/brping/pingmessage.py
@@ -41,7 +41,8 @@ class PingMessage(object):
     ## number of bytes in a checksum
     checksumLength = 2
 
-    ## Messge constructor
+    ## Message constructor
+    # Initialize from provided data (for packing and transmitting)
     #
     # @par Ex request:
     # @code
@@ -53,21 +54,11 @@ class PingMessage(object):
     #
     # @par Ex set:
     # @code
-    # m = PingMessage(Ping1dMessage.SET_RANGE)
-    # m.start_mm = 1000
-    # m.length_mm = 2000
-    # m.update_checksum()
+    # m = PingMessage(Ping1dMessage.SET_RANGE, start_mm=1000, length_mm=2000)
+    # m.pack_msg_data()
     # write(m.msg_data)
     # @endcode
-    #
-    # @par Ex receive:
-    # @code
-    # m = PingMessage(rxByteArray)
-    # if m.message_id == Ping1dMessage.RANGE
-    #     start_mm = m.start_mm
-    #     length_mm = m.length_mm
-    # @endcode
-    def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_fields):
+    def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_data):
         ## The message id
         self.message_id = msg_id
 
@@ -85,9 +76,6 @@ def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_fields)
         # update with pack_msg_data()
         self.msg_data = None
 
-        # Constructor 2: make a pingmessage object cooresponding to a message
-        # id, with field members ready to access and populate
-        # (for packing + transmitting)
         try:
             ## The name of this message
             self.name = payload_dict[self.message_id].name
@@ -97,7 +85,7 @@ def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_fields)
 
             # initialize payload field members
             for attr in self.payload_field_names:
-                setattr(self, attr, payload_fields.get(attr, 0))
+                setattr(self, attr, payload_data.get(attr, 0))
 
             # initialize vector field if present in message
             if self.message_id in variable_msgs:
@@ -112,13 +100,27 @@ def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_fields)
             ## The struct formatting string for the message payload
             self.payload_format = self.get_payload_format()
 
+            # TODO pack_msg_data
+            # Either when data is provided, or with a 'pack' argument (default True?)
+            # - avoid re-updating payload length
+            # - probably change get_payload_format to update_payload_format
+
         except KeyError as e:
             message_id = self.message_id
             raise Exception(f"{message_id = } not recognized\n{msg_data = }") from e
 
+    ## Alternate constructor
+    # Initialize from a binary data buffer
+    #
+    # @par Ex receive:
+    # @code
+    # m = PingMessage.from_buffer(rxByteArray)
+    # if m.message_id == Ping1dMessage.RANGE
+    #     start_mm = m.start_mm
+    #     length_mm = m.length_mm
+    # @endcode
     @classmethod
     def from_buffer(cls, msg_data):
-        """ Alternate constructor - initialise from a binary data buffer. """
         msg = cls()
         if not msg.unpack_msg_data(msg_data):
             # Attempted to create an unknown message

From 09cda7e5b49af019b3d739a59676f1360e6712cc Mon Sep 17 00:00:00 2001
From: ES-Alexander <sandman.esalexander@gmail.com>
Date: Fri, 15 Apr 2022 16:17:20 +1000
Subject: [PATCH 15/15] pingmessage: improve update functions

---
 brping/pingmessage.py | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/brping/pingmessage.py b/brping/pingmessage.py
index d4ecf33..9ab4b1a 100644
--- a/brping/pingmessage.py
+++ b/brping/pingmessage.py
@@ -98,12 +98,11 @@ def __init__(self, msg_id=0, dst_device_id=0, src_device_id=0, **payload_data):
             self.update_payload_length()
 
             ## The struct formatting string for the message payload
-            self.payload_format = self.get_payload_format()
+            self.update_payload_format()
 
             # TODO pack_msg_data
             # Either when data is provided, or with a 'pack' argument (default True?)
             # - avoid re-updating payload length
-            # - probably change get_payload_format to update_payload_format
 
         except KeyError as e:
             message_id = self.message_id
@@ -135,7 +134,8 @@ def pack_msg_data(self):
         self.update_payload_length()
 
         # Prepare struct packing format string
-        msg_format = self.endianess + self.header_format + self.get_payload_format()
+        self.update_payload_format()
+        msg_format = self.endianess + self.header_format + self.payload_format
 
         # Prepare complete list of field names (header + payload)
         attrs = self.header_field_names + payload_dict[self.message_id].field_names
@@ -180,7 +180,7 @@ def unpack_msg_data(self, msg_data):
 
         if self.payload_length > 0:
             ## The struct formatting string for the message payload
-            self.payload_format = self.get_payload_format()
+            self.update_payload_format()
 
             # Extract payload
             try:
@@ -220,24 +220,23 @@ def verify_checksum(self):
 
     ## Update the payload_length attribute with the **current** payload length, including dynamic length fields (if present)
     def update_payload_length(self):
+        self.payload_length = payload_dict[self.message_id].payload_length
+
         if self.message_id in variable_msgs or self.message_id in asciiMsgs:
             # The last field self.payload_field_names[-1] is always the single dynamic-length field
-            self.payload_length = payload_dict[self.message_id].payload_length + len(getattr(self, self.payload_field_names[-1]))
-        else:
-            self.payload_length = payload_dict[self.message_id].payload_length
+            self.payload_length += len(getattr(self, self.payload_field_names[-1]))
 
-    ## Get the python struct formatting string for the message payload
-    # @return the payload struct format string
-    def get_payload_format(self):
+    ## Update the python struct formatting string for the message payload
+    def update_payload_format(self):
         # messages with variable length fields
         if self.message_id in variable_msgs or self.message_id in asciiMsgs:
-            var_length = self.payload_length - payload_dict[self.message_id].payload_length  # Subtract static length portion from payload length
-            if var_length <= 0:
-                return payload_dict[self.message_id].format  # variable data portion is empty
-
-            return payload_dict[self.message_id].format + str(var_length) + "s"
-        else: # messages with a static (constant) length
-            return payload_dict[self.message_id].format
+            # Subtract static length portion from payload length
+            var_length = self.payload_length - payload_dict[self.message_id].payload_length
+            if var_length > 0:
+                self.payload_format = payload_dict[self.message_id].format + str(var_length) + "s"
+                return
+        # messages with a static (constant) length, or empty data portion
+        self.payload_format = payload_dict[self.message_id].format
 
     ## Dump object into string representation
     # @return string representation of the object