From d72f6292354f11517818f00de4cc6deb6725c14b Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Thu, 20 Apr 2017 16:42:40 +1000
Subject: [PATCH 01/15] Initial version of named struct class.

---
 instruments/named_struct.py | 197 ++++++++++++++++++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 instruments/named_struct.py

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
new file mode 100644
index 000000000..0a3ffbff7
--- /dev/null
+++ b/instruments/named_struct.py
@@ -0,0 +1,197 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+"""
+Class for quickly defining C-like structures with named fields.
+"""
+
+# IMPORTS #####################################################################
+
+from __future__ import absolute_import
+from __future__ import division
+
+import struct
+from collections import OrderedDict
+
+from future.utils import with_metaclass
+
+# DESIGN NOTES ################################################################
+"""
+This class uses the Django-like strategy described at
+    http://stackoverflow.com/a/3288988/267841
+to assign a "birthday" to each Field as it's instantiated. We can thus sort
+each Field in a NamedStruct by its birthday.
+
+Notably, this hack is not at all required on Python 3.6:
+    https://www.python.org/dev/peps/pep-0520/
+
+TODO: arrays other than string arrays do not currently work.
+"""
+
+# CLASSES #####################################################################
+
+class Field(object):
+    """
+    A named field within a C-style structure.
+
+    :param str fmt: Format for the field, corresponding to the
+        documentation of the :mod:`struct` standard library package.
+    """
+
+    __n_fields_created = 0
+    _field_birthday = None
+
+    _fmt = ''
+    _name = None
+    _owner_type = object
+
+    def __init__(self, fmt):
+        super(Field, self).__init__()
+
+        # Record our birthday so that we can sort fields later.
+        self._field_birthday = Field.__n_fields_created
+        Field.__n_fields_created += 1
+
+        self._fmt = fmt
+
+    def is_significant(self):
+        return not self._fmt.strip().endswith('x')
+
+    def __repr__(self):
+        if self._owner_type:
+            return "<Field {} of {}, fmt={}>".format(
+                self._name, self._owner_type, self._fmt
+            )
+
+        return "<Unbound field, fmt={}>".format(
+            self._fmt
+        )
+
+    def __str__(self):
+        fmt = self._fmt.strip()
+        n, fmt_char = fmt[:-1], fmt[-1]
+        c_type = {
+            'x': 'char',
+            'c': 'char',
+            'b': 'char',
+            'B': 'unsigned char',
+            '?': 'bool',
+            'h': 'short',
+            'H': 'unsigned short',
+            'i': 'int',
+            'I': 'unsigned int',
+            'l': 'long',
+            'L': 'unsigned long',
+            'q': 'long long',
+            'Q': 'unsigned long long',
+            'f': 'float',
+            'd': 'double',
+            # NB: no [], since that will be implied by n.
+            's': 'char',
+            'p': 'char',
+            'P': 'void *'
+        }[fmt_char]
+
+        if n:
+            c_type = "{}[{}]".format(c_type, n)
+        return (
+            "{c_type} {self._name}".format(c_type=c_type, self=self)
+            if self.is_significant()
+            else c_type
+        )
+
+    # DESCRIPTOR PROTOCOL #
+
+    def __get__(self, obj, type=None):
+        return obj._values[self._name]
+
+    def __set__(self, obj, value):
+        obj._values[self._name] = value
+
+class Padding(Field):
+    def __init__(self, n_bytes=1):
+        super(Padding, self).__init__('{}x'.format(n_bytes))
+
+class HasFields(type):
+    def __new__(mcls, name, bases, attrs):
+        # Since this is a metaclass, the __new__ method observes
+        # creation of new *classes* and not new instances.
+        # We call the superclass of HasFields, which is another
+        # metaclass, to do most of the heavy lifting of creating
+        # the new class.
+        cls = super(HasFields, mcls).__new__(mcls, name, bases, attrs)
+
+        # We now sort the fields by their birthdays and store them in an
+        # ordered dict for easier look up later.
+        cls._fields = OrderedDict([
+            (field_name, field)
+            for field_name, field in sorted(
+                [
+                    (field_name, field)
+                    for field_name, field in attrs.items()
+                    if isinstance(field, Field)
+                ],
+                key=lambda item: item[1]._field_birthday
+            )
+        ])
+
+        # Assign names and owner types to each field so that they can follow
+        # the descriptor protocol.
+        for field_name, field in cls._fields.items():
+            field._name = field_name
+            field._owner_type = cls
+
+        # Associate a struct.Struct instance with the new class
+        # that defines how to pack/unpack the new type.
+        cls._struct = struct.Struct(
+            # TODO: support alignment char at start.
+            " ".join([
+                field._fmt for field in cls._fields.values()
+            ])
+        )
+
+        return cls
+
+class NamedStruct(with_metaclass(HasFields, object)):
+    def __init__(self, **kwargs):
+        super(NamedStruct, self).__init__()
+        self._values = OrderedDict([
+            (
+                field._name,
+                kwargs[field._name] if field._name in kwargs else None
+            )
+            for field in filter(Field.is_significant, self._fields.values())
+        ])
+
+    def _to_seq(self):
+        return tuple(self._values.values())
+
+    @classmethod
+    def _from_seq(cls, new_values):
+        return cls(**{
+            field._name: new_value
+            for field, new_value in
+            zip(filter(Field.is_significant, cls._fields.values()), new_values)
+        })
+
+    def pack(self):
+        return self._struct.pack(*self._to_seq())
+
+    @classmethod
+    def unpack(cls, buffer):
+        return cls._from_seq(cls._struct.unpack(buffer))
+
+    def __str__(self):
+        return "{name} {{\n{fields}\n}}".format(
+            name=type(self).__name__,
+            fields="\n".join([
+                "    {field}{value};".format(
+                    field=field,
+                    value=(
+                        " = {}".format(repr(self._values[field._name]))
+                        if field.is_significant()
+                        else ""
+                    )
+                )
+                for field in self._fields.values()
+            ])
+        )

From b5fdc5abdd6453a74e2444e3a7399cd19c0e5c9d Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Thu, 20 Apr 2017 17:00:48 +1000
Subject: [PATCH 02/15] Added tests for new class.

---
 instruments/named_struct.py            | 59 +++++++++++++++++++++++++-
 instruments/tests/test_named_struct.py | 26 ++++++++++++
 2 files changed, 83 insertions(+), 2 deletions(-)
 create mode 100644 instruments/tests/test_named_struct.py

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 0a3ffbff7..99eff2c2b 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -27,6 +27,13 @@
 TODO: arrays other than string arrays do not currently work.
 """
 
+# PYLINT CONFIGURATION ########################################################
+
+# All of the classes in this module need to interact with each other rather
+# deeply, so we disable the protected-access check within this module.
+
+# pylint:disable=protected-access
+
 # CLASSES #####################################################################
 
 class Field(object):
@@ -112,13 +119,13 @@ def __init__(self, n_bytes=1):
         super(Padding, self).__init__('{}x'.format(n_bytes))
 
 class HasFields(type):
-    def __new__(mcls, name, bases, attrs):
+    def __new__(mcs, name, bases, attrs):
         # Since this is a metaclass, the __new__ method observes
         # creation of new *classes* and not new instances.
         # We call the superclass of HasFields, which is another
         # metaclass, to do most of the heavy lifting of creating
         # the new class.
-        cls = super(HasFields, mcls).__new__(mcls, name, bases, attrs)
+        cls = super(HasFields, mcs).__new__(mcs, name, bases, attrs)
 
         # We now sort the fields by their birthdays and store them in an
         # ordered dict for easier look up later.
@@ -152,6 +159,34 @@ def __new__(mcls, name, bases, attrs):
         return cls
 
 class NamedStruct(with_metaclass(HasFields, object)):
+    """
+    Represents a C-style struct with one or more named fields,
+    useful for packing and unpacking serialized data documented
+    in terms of C examples. For instance, consider a struct of the
+    form::
+
+        typedef struct {
+            unsigned long a = 0x1234;
+            char[12] dummy;
+            unsigned char b = 0xab;
+        } Foo;
+
+    This struct can be represented as the following NamedStruct::
+
+        class Foo(NamedStruct):
+            a = Field('L')
+            dummy = Padding(12)
+            b = Field('B')
+
+        foo = Foo(a=0x1234, b=0xab)
+    """
+
+    # Provide reasonable defaults for the lowercase-f-fields
+    # created by HasFields. This will prevent a few edge cases,
+    # allow type inference and will prevent pylint false positives.
+    _fields = {}
+    _struct = None
+
     def __init__(self, **kwargs):
         super(NamedStruct, self).__init__()
         self._values = OrderedDict([
@@ -174,12 +209,32 @@ def _from_seq(cls, new_values):
         })
 
     def pack(self):
+        """
+        Packs this instance into bytes, suitable for transmitting over
+        a network or recording to disc. See :func:`struct.pack` for details.
+
+        :return bytes packed_data: A serialized representation of this
+            instance.
+        """
         return self._struct.pack(*self._to_seq())
 
     @classmethod
     def unpack(cls, buffer):
+        """
+        Given a buffer, unpacks it into an instance of this NamedStruct.
+        See :func:`struct.unpack` for details.
+
+        :param bytes buffer: Data to use in creating a new instance.
+        :return: The new instance represented by `buffer`.
+        """
         return cls._from_seq(cls._struct.unpack(buffer))
 
+    def __eq__(self, other):
+        if not isinstance(other, NamedStruct):
+            return False
+
+        return self._values == other._values
+
     def __str__(self):
         return "{name} {{\n{fields}\n}}".format(
             name=type(self).__name__,
diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
new file mode 100644
index 000000000..c32a116c0
--- /dev/null
+++ b/instruments/tests/test_named_struct.py
@@ -0,0 +1,26 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+"""
+Module containing tests for named structures.
+"""
+
+# IMPORTS ####################################################################
+
+from __future__ import absolute_import
+
+from instruments.named_struct import (
+    Field, Padding, NamedStruct
+)
+
+# TESTS ######################################################################
+
+# pylint: disable=no-member,protected-access,blacklisted-name,missing-docstring
+
+def test_named_struct_roundtrip():
+    class Foo(NamedStruct):
+        a = Field('H')
+        padding = Padding(12)
+        b = Field('B')
+
+    foo = Foo(a=0x1234, b=0xab)
+    assert Foo.unpack(foo.pack()) == foo

From 8a89bf808b39c19530a44d045733fd428df1f5e0 Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Thu, 20 Apr 2017 17:07:10 +1000
Subject: [PATCH 03/15] docs and a pylint fix.

---
 doc/source/devguide/util_fns.rst | 12 ++++++++++++
 instruments/named_struct.py      | 17 ++++++++---------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/doc/source/devguide/util_fns.rst b/doc/source/devguide/util_fns.rst
index 2d2719a91..9fa06769b 100644
--- a/doc/source/devguide/util_fns.rst
+++ b/doc/source/devguide/util_fns.rst
@@ -183,3 +183,15 @@ String Property
 .. autofunction:: string_property
 
 
+Named Structures
+================
+
+The :class:`~instruments.named_struct.NamedStruct` class can be used to represent
+C-style structures for serializing and deserializing data.
+
+.. autoclass:: instruments.named_struct.NamedStruct
+
+.. autoclass:: instruments.named_struct.Field
+
+.. autoclass:: instruments.named_struct.Padding
+
diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 99eff2c2b..462c33165 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -15,17 +15,16 @@
 from future.utils import with_metaclass
 
 # DESIGN NOTES ################################################################
-"""
-This class uses the Django-like strategy described at
-    http://stackoverflow.com/a/3288988/267841
-to assign a "birthday" to each Field as it's instantiated. We can thus sort
-each Field in a NamedStruct by its birthday.
 
-Notably, this hack is not at all required on Python 3.6:
-    https://www.python.org/dev/peps/pep-0520/
+# This class uses the Django-like strategy described at
+#     http://stackoverflow.com/a/3288988/267841
+# to assign a "birthday" to each Field as it's instantiated. We can thus sort
+# each Field in a NamedStruct by its birthday.
 
-TODO: arrays other than string arrays do not currently work.
-"""
+# Notably, this hack is not at all required on Python 3.6:
+#     https://www.python.org/dev/peps/pep-0520/
+
+# TODO: arrays other than string arrays do not currently work.
 
 # PYLINT CONFIGURATION ########################################################
 

From fc177be465ffdfb3f418f08eb23851cc9dc694bb Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Thu, 20 Apr 2017 17:11:40 +1000
Subject: [PATCH 04/15] =?UTF-8?q?Py3k=20fix=20(filter=20=E2=86=92=20list(f?=
 =?UTF-8?q?ilter))?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 instruments/named_struct.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 462c33165..9449366d4 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -204,7 +204,7 @@ def _from_seq(cls, new_values):
         return cls(**{
             field._name: new_value
             for field, new_value in
-            zip(filter(Field.is_significant, cls._fields.values()), new_values)
+            zip(list(filter(Field.is_significant, cls._fields.values())), new_values)
         })
 
     def pack(self):

From 348545b44d2304e53a52e96b332749f1e4b45cf4 Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Thu, 20 Apr 2017 17:48:11 +1000
Subject: [PATCH 05/15] Improvements to string handling.

---
 instruments/named_struct.py            | 70 +++++++++++++++++++++++---
 instruments/tests/test_named_struct.py | 11 +++-
 2 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 9449366d4..0ac5f29c5 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -50,17 +50,32 @@ class Field(object):
     _name = None
     _owner_type = object
 
-    def __init__(self, fmt):
+    def __init__(self, fmt, strip_null=False):
         super(Field, self).__init__()
 
         # Record our birthday so that we can sort fields later.
         self._field_birthday = Field.__n_fields_created
         Field.__n_fields_created += 1
 
-        self._fmt = fmt
+        self._fmt = fmt.strip()
+        self._strip_null = strip_null
 
     def is_significant(self):
-        return not self._fmt.strip().endswith('x')
+        return not self._fmt.endswith('x')
+
+    @property
+    def fmt_char(self):
+        return self._fmt[-1]
+
+    def __len__(self):
+        if self._fmt[:-1]:
+            length = int(self._fmt[-1])
+            if length < 0:
+                raise TypeError("Field is specified with negative length.")
+
+            return length
+
+        raise TypeError("Field is scalar and has no len().")
 
     def __repr__(self):
         if self._owner_type:
@@ -73,8 +88,7 @@ def __repr__(self):
         )
 
     def __str__(self):
-        fmt = self._fmt.strip()
-        n, fmt_char = fmt[:-1], fmt[-1]
+        n, fmt_char = len(self), self.fmt_char
         c_type = {
             'x': 'char',
             'c': 'char',
@@ -113,7 +127,47 @@ def __get__(self, obj, type=None):
     def __set__(self, obj, value):
         obj._values[self._name] = value
 
+class StringField(Field):
+    """
+    Represents a field that is interpreted as a Python string.
+
+    :param int length: Maximum allowed length of the field, as
+        measured in the number of bytes used by its encoding.
+        Note that if a shorter string is provided, it will
+        be padded by null bytes.
+    :param str encoding: Name of an encoding to use in serialization
+        and deserialization to Python strings.
+    :param bool strip_null: If `True`, null bytes (``'\x00'``) will
+        be removed from the right upon deserialization.
+    """
+
+    _strip_null = False
+    _encoding = 'ascii'
+
+    def __init__(self, length, encoding='ascii', strip_null=False):
+        super(StringField, self).__init__('{}s'.format(length))
+        self._strip_null = strip_null
+        self._encoding = encoding
+
+    def __set__(self, obj, value):
+        if self._strip_null:
+            value = value.rstrip('\x00')
+        value = value.encode(self._encoding)
+
+        super(StringField, self).__set__(obj, value)
+
+    def __get__(self, obj, type=None):
+        super(StringField, self).__get__(obj, type=type).decode(self._encoding)
+
+
 class Padding(Field):
+    """
+    Represents a field whose value is insignificant, and will not
+    be kept in serialization and deserialization.
+
+    :param int n_bytes: Number of padding bytes occupied by this field.
+    """
+
     def __init__(self, n_bytes=1):
         super(Padding, self).__init__('{}x'.format(n_bytes))
 
@@ -190,12 +244,14 @@ def __init__(self, **kwargs):
         super(NamedStruct, self).__init__()
         self._values = OrderedDict([
             (
-                field._name,
-                kwargs[field._name] if field._name in kwargs else None
+                field._name, None
             )
             for field in filter(Field.is_significant, self._fields.values())
         ])
 
+        for field_name, value in kwargs.items():
+            setattr(self, field_name, value)
+
     def _to_seq(self):
         return tuple(self._values.values())
 
diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index c32a116c0..426daa5f3 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -9,7 +9,7 @@
 from __future__ import absolute_import
 
 from instruments.named_struct import (
-    Field, Padding, NamedStruct
+    Field, StringField, Padding, NamedStruct
 )
 
 # TESTS ######################################################################
@@ -24,3 +24,12 @@ class Foo(NamedStruct):
 
     foo = Foo(a=0x1234, b=0xab)
     assert Foo.unpack(foo.pack()) == foo
+
+
+def test_named_struct_str():
+    class Foo(NamedStruct):
+        a = StringField(8)
+        b = StringField(9, strip_null=True)
+
+    foo = Foo(a="0123456\x00", b='abc')
+    assert Foo.unpack(foo.pack()) == foo

From b12da66f11d81626133f8489be9fe9306207e950 Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Thu, 20 Apr 2017 17:49:01 +1000
Subject: [PATCH 06/15] Ignored additional carp.

---
 .gitignore | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/.gitignore b/.gitignore
index 3f7a79c94..fedb392fa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -50,3 +50,9 @@ nosetests.xml
 
 #pycharm generated
 .idea
+
+# VS Code IDE internals
+.vscode/
+
+# nosetests metadata
+.noseids

From 6dd744e9f6f8fb50db3c242979c68fc49fe1952d Mon Sep 17 00:00:00 2001
From: Steven Casagrande <scasagrande@galvant.ca>
Date: Sat, 22 Apr 2017 12:30:22 -0400
Subject: [PATCH 07/15] Provide fixes for build and start using hypothesis

---
 .gitignore                             | 3 +++
 dev-requirements.txt                   | 1 +
 instruments/named_struct.py            | 8 +++++++-
 instruments/tests/test_named_struct.py | 9 +++++++--
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index fedb392fa..3d527fd94 100644
--- a/.gitignore
+++ b/.gitignore
@@ -56,3 +56,6 @@ nosetests.xml
 
 # nosetests metadata
 .noseids
+
+# Hypothesis files
+.hypothesis/
diff --git a/dev-requirements.txt b/dev-requirements.txt
index c42f8689e..52fbf4786 100644
--- a/dev-requirements.txt
+++ b/dev-requirements.txt
@@ -1,3 +1,4 @@
 mock
 nose
 pylint==1.6.5
+hypothesis
diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 0ac5f29c5..13a3ae1f0 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -35,6 +35,7 @@
 
 # CLASSES #####################################################################
 
+
 class Field(object):
     """
     A named field within a C-style structure.
@@ -73,7 +74,9 @@ def __len__(self):
             if length < 0:
                 raise TypeError("Field is specified with negative length.")
 
-            return length
+            # Although we know that length>0, this abs ensures that static
+            # code checks are happy with __len__ always returning a positive number
+            return abs(length)
 
         raise TypeError("Field is scalar and has no len().")
 
@@ -150,6 +153,8 @@ def __init__(self, length, encoding='ascii', strip_null=False):
         self._encoding = encoding
 
     def __set__(self, obj, value):
+        if isinstance(value, bytes):
+            value = value.decode(self._encoding)
         if self._strip_null:
             value = value.rstrip('\x00')
         value = value.encode(self._encoding)
@@ -211,6 +216,7 @@ def __new__(mcs, name, bases, attrs):
 
         return cls
 
+
 class NamedStruct(with_metaclass(HasFields, object)):
     """
     Represents a C-style struct with one or more named fields,
diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index 426daa5f3..a51df9827 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -8,6 +8,9 @@
 
 from __future__ import absolute_import
 
+from hypothesis import given
+import hypothesis.strategies as st
+
 from instruments.named_struct import (
     Field, StringField, Padding, NamedStruct
 )
@@ -16,13 +19,15 @@
 
 # pylint: disable=no-member,protected-access,blacklisted-name,missing-docstring
 
-def test_named_struct_roundtrip():
+
+@given(st.integers(min_value=0, max_value=0x7FFF*2+1), st.integers(min_value=0, max_value=0xFF))
+def test_named_struct_roundtrip(var1, var2):
     class Foo(NamedStruct):
         a = Field('H')
         padding = Padding(12)
         b = Field('B')
 
-    foo = Foo(a=0x1234, b=0xab)
+    foo = Foo(a=var1, b=var2)
     assert Foo.unpack(foo.pack()) == foo
 
 

From a897b8c5552121eaaccea59806d52426cfe7430d Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 12:19:33 +1000
Subject: [PATCH 08/15] Locked requirements.txt; see #174.

---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index 30a1a88d6..f76e8b075 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
-numpy
+numpy==1.12.1
 pyserial
 quantities
 future>=0.15

From 50733ef04772fb169ae8a2d0ed853d263e630fbb Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 12:42:35 +1000
Subject: [PATCH 09/15] Fixed py3k compat issue.

---
 instruments/named_struct.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 13a3ae1f0..195ed8591 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -296,6 +296,9 @@ def __eq__(self, other):
 
         return self._values == other._values
 
+    def __hash__(self):
+        return hash(self._values)
+
     def __str__(self):
         return "{name} {{\n{fields}\n}}".format(
             name=type(self).__name__,

From 3f14e6b8ecd7963394ed23f4fd65fc0f17e14683 Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 12:56:30 +1000
Subject: [PATCH 10/15] Fixed negative length checks.

---
 instruments/named_struct.py            | 12 ++++----
 instruments/tests/test_named_struct.py | 39 +++++++++++++++++---------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index 195ed8591..fb9844476 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -61,6 +61,12 @@ def __init__(self, fmt, strip_null=False):
         self._fmt = fmt.strip()
         self._strip_null = strip_null
 
+        # If we're given a length, check that it
+        # makes sense.
+        if self._fmt[:-1] and int(self._fmt[:-1]) < 0:
+            raise TypeError("Field is specified with negative length.")
+
+
     def is_significant(self):
         return not self._fmt.endswith('x')
 
@@ -70,11 +76,7 @@ def fmt_char(self):
 
     def __len__(self):
         if self._fmt[:-1]:
-            length = int(self._fmt[-1])
-            if length < 0:
-                raise TypeError("Field is specified with negative length.")
-
-            # Although we know that length>0, this abs ensures that static
+            # Although we know that length > 0, this abs ensures that static
             # code checks are happy with __len__ always returning a positive number
             return abs(length)
 
diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index a51df9827..d1dd66299 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -11,6 +11,8 @@
 from hypothesis import given
 import hypothesis.strategies as st
 
+from unittest import TestCase
+
 from instruments.named_struct import (
     Field, StringField, Padding, NamedStruct
 )
@@ -19,22 +21,31 @@
 
 # pylint: disable=no-member,protected-access,blacklisted-name,missing-docstring
 
+class TestNamedStruct(TestCase):
+    @given(st.integers(min_value=0, max_value=0x7FFF*2+1), st.integers(min_value=0, max_value=0xFF))
+    def test_roundtrip(self, var1, var2):
+        class Foo(NamedStruct):
+            a = Field('H')
+            padding = Padding(12)
+            b = Field('B')
+
+        foo = Foo(a=var1, b=var2)
+        assert Foo.unpack(foo.pack()) == foo
 
-@given(st.integers(min_value=0, max_value=0x7FFF*2+1), st.integers(min_value=0, max_value=0xFF))
-def test_named_struct_roundtrip(var1, var2):
-    class Foo(NamedStruct):
-        a = Field('H')
-        padding = Padding(12)
-        b = Field('B')
 
-    foo = Foo(a=var1, b=var2)
-    assert Foo.unpack(foo.pack()) == foo
+    def test_str(self):
+        class Foo(NamedStruct):
+            a = StringField(8)
+            b = StringField(9, strip_null=True)
 
+        foo = Foo(a="0123456\x00", b='abc')
+        assert Foo.unpack(foo.pack()) == foo
 
-def test_named_struct_str():
-    class Foo(NamedStruct):
-        a = StringField(8)
-        b = StringField(9, strip_null=True)
 
-    foo = Foo(a="0123456\x00", b='abc')
-    assert Foo.unpack(foo.pack()) == foo
+    def test_negative_len(self):
+        """
+        Checks whether negative field lengths correctly raise.
+        """
+        with self.assertRaises(TypeError):
+            class Foo(NamedStruct):
+                a = StringField(-1)

From ba07ae8b2cd8f81a322d247cc38a540df515f5c5 Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 13:14:40 +1000
Subject: [PATCH 11/15] Fixed bug in StringField.__get__

---
 instruments/named_struct.py            |  4 ++--
 instruments/tests/test_named_struct.py | 12 +++++++++---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/instruments/named_struct.py b/instruments/named_struct.py
index fb9844476..ef8522ab7 100644
--- a/instruments/named_struct.py
+++ b/instruments/named_struct.py
@@ -78,7 +78,7 @@ def __len__(self):
         if self._fmt[:-1]:
             # Although we know that length > 0, this abs ensures that static
             # code checks are happy with __len__ always returning a positive number
-            return abs(length)
+            return abs(int(self._fmt[:-1]))
 
         raise TypeError("Field is scalar and has no len().")
 
@@ -164,7 +164,7 @@ def __set__(self, obj, value):
         super(StringField, self).__set__(obj, value)
 
     def __get__(self, obj, type=None):
-        super(StringField, self).__get__(obj, type=type).decode(self._encoding)
+        return super(StringField, self).__get__(obj, type=type).decode(self._encoding)
 
 
 class Padding(Field):
diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index d1dd66299..db1c1719f 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -6,7 +6,7 @@
 
 # IMPORTS ####################################################################
 
-from __future__ import absolute_import
+from __future__ import absolute_import, unicode_literals
 
 from hypothesis import given
 import hypothesis.strategies as st
@@ -35,12 +35,18 @@ class Foo(NamedStruct):
 
     def test_str(self):
         class Foo(NamedStruct):
-            a = StringField(8)
+            a = StringField(8, strip_null=False)
             b = StringField(9, strip_null=True)
+            c = StringField(2, encoding='utf-8')
 
-        foo = Foo(a="0123456\x00", b='abc')
+        foo = Foo(a="0123456\x00", b='abc', c=u'α')
         assert Foo.unpack(foo.pack()) == foo
 
+        # Also check that we can get fields out directly.
+        self.assertEqual(foo.a, '0123456\x00')
+        self.assertEqual(foo.b, 'abc')
+        self.assertEqual(foo.c, u'α')
+
 
     def test_negative_len(self):
         """

From cd3979c55f5184452e72c12b92c8decd103a45df Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 13:17:41 +1000
Subject: [PATCH 12/15] Added NamedStruct.__eq__ test.

---
 instruments/tests/test_named_struct.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index db1c1719f..8dbb0a0c0 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -55,3 +55,15 @@ def test_negative_len(self):
         with self.assertRaises(TypeError):
             class Foo(NamedStruct):
                 a = StringField(-1)
+
+    def test_equality(self):
+        class Foo(NamedStruct):
+            a = Field('H')
+            b = Field('B')
+            c = StringField(5, encoding='utf8', strip_null=True)
+
+        foo1 = Foo(a=0x1234, b=0x56, c=u'ω')
+        foo2 = Foo(a=0xabcd, b=0xef, c=u'α')
+
+        assert foo1 == foo1
+        assert foo1 != foo2

From 5c03d80a1f66b09095d6729a7f6f0ce9ae00d418 Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 13:23:50 +1000
Subject: [PATCH 13/15] Fixing pylint warnings.

---
 instruments/tests/test_named_struct.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index 8dbb0a0c0..3997c39e1 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -8,18 +8,19 @@
 
 from __future__ import absolute_import, unicode_literals
 
+from unittest import TestCase
+
 from hypothesis import given
 import hypothesis.strategies as st
 
-from unittest import TestCase
-
 from instruments.named_struct import (
     Field, StringField, Padding, NamedStruct
 )
 
 # TESTS ######################################################################
 
-# pylint: disable=no-member,protected-access,blacklisted-name,missing-docstring
+# We disable pylint warnings that are not as applicable for unit tests.
+# pylint: disable=no-member,protected-access,blacklisted-name,missing-docstring,no-self-use
 
 class TestNamedStruct(TestCase):
     @given(st.integers(min_value=0, max_value=0x7FFF*2+1), st.integers(min_value=0, max_value=0xFF))
@@ -48,7 +49,7 @@ class Foo(NamedStruct):
         self.assertEqual(foo.c, u'α')
 
 
-    def test_negative_len(self):
+    def test_negative_len(self): # pylint: disable=unused-variable
         """
         Checks whether negative field lengths correctly raise.
         """

From 165a7dd46d89d68e293f9d102481c0e694d52729 Mon Sep 17 00:00:00 2001
From: Christopher Granade <cgranade@cgranade.com>
Date: Wed, 12 Jul 2017 14:30:26 +1000
Subject: [PATCH 14/15] Moved pylint warning comment.

---
 instruments/tests/test_named_struct.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/instruments/tests/test_named_struct.py b/instruments/tests/test_named_struct.py
index 3997c39e1..adeef46da 100644
--- a/instruments/tests/test_named_struct.py
+++ b/instruments/tests/test_named_struct.py
@@ -49,12 +49,12 @@ class Foo(NamedStruct):
         self.assertEqual(foo.c, u'α')
 
 
-    def test_negative_len(self): # pylint: disable=unused-variable
+    def test_negative_len(self):
         """
         Checks whether negative field lengths correctly raise.
         """
         with self.assertRaises(TypeError):
-            class Foo(NamedStruct):
+            class Foo(NamedStruct): # pylint: disable=unused-variable
                 a = StringField(-1)
 
     def test_equality(self):

From 8c066fb34e89e81c509052048cd74024780feb9d Mon Sep 17 00:00:00 2001
From: Chris Granade <cgranade@cgranade.com>
Date: Fri, 14 Jul 2017 14:10:07 +1000
Subject: [PATCH 15/15] Changed numpy version in reqs as suggested by
 @scasagrande

---
 requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index f76e8b075..89e22a811 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,4 +1,4 @@
-numpy==1.12.1
+numpy<1.13.0
 pyserial
 quantities
 future>=0.15