Merge pull request #445 from macartur/binarydata_fixes

Binarydata fixes

Author: beraldoleal

pyof/foundation/basic_types.py

@@ -428,18 +428,12 @@ class BinaryData(GenericType):
     return the size of the instance using Python's :func:`len`.
     """
 
-    def __init__(self, value=b''):  # noqa
+    def __init__(self, value=None):  # pylint: disable=useless-super-delegation
         """The constructor takes the parameter below.
 
         Args:
             value (bytes): The binary data. Defaults to an empty value.
-
-        Raises:
-            ValueError: If given value is not bytes.
-
         """
-        if not isinstance(value, bytes):
-            raise ValueError('BinaryData must contain bytes.')
         super().__init__(value)
 
     def pack(self, value=None):
@@ -449,21 +443,20 @@ def pack(self, value=None):
             bytes: The binary representation.
 
         Raises:
-            :exc:`~.exceptions.NotBinaryData`: If value is not :class:`bytes`.
+            ValueError: If value can't be represented with bytes
 
         """
-        if isinstance(value, type(self)):
-            return value.pack()
-
         if value is None:
             value = self._value
 
-        if value:
-            if isinstance(value, bytes):
-                return value
-            raise ValueError('BinaryData must contain bytes.')
-
-        return b''
+        if hasattr(value, 'pack') and callable(value.pack):
+            return value.pack()
+        elif isinstance(value, bytes):
+            return value
+        elif value is None:
+            return b''
+        else:
+            raise ValueError(f"BinaryData can't be {type(value)} = '{value}'")
 
     def unpack(self, buff, offset=0):
         """Unpack a binary message into this object's attributes.
@@ -490,11 +483,12 @@ class attribute of the struct.
 
         """
         if value is None:
-            return len(self._value)
-        elif hasattr(value, 'get_size'):
+            value = self._value
+
+        if hasattr(value, 'get_size'):
             return value.get_size()
 
-        return len(value)
+        return len(self.pack(value))
 
 
 class TypeList(list, GenericStruct):

tests/test_foundation/test_basic_types.py

@@ -172,7 +172,13 @@ def test_default_value(self):
         actual = BinaryData().pack()
         self.assertEqual(expected, actual)
 
-    def test_pack_bytes(self):
+    def test_pack_none_value(self):
+        """Test packing None value."""
+        expected = b''
+        actual = BinaryData(None).pack()
+        self.assertEqual(expected, actual)
+
+    def test_pack_bytes_value(self):
         """Test packing some bytes."""
         expected = b'forty two'
         actual = BinaryData(expected).pack()
@@ -184,10 +190,14 @@ def test_pack_empty_bytes(self):
         actual = BinaryData(expected).pack()
         self.assertEqual(expected, actual)
 
-    def test_unexpected_value(self):
-        """Should raise ValueError if constructor value is not bytes."""
-        self.assertRaises(ValueError, BinaryData, "can't be string")
+    def test_pack_packable_value(self):
+        """Test packing packable value."""
+        hw_addr = basic_types.HWAddress('0a:d3:98:a5:30:47')
+        expected = hw_addr.pack()
+        actual = BinaryData(hw_addr).pack()
+        self.assertEqual(expected, actual)
 
     def test_unexpected_value_as_parameter(self):
         """Should raise ValueError if pack value is not bytes."""
-        self.assertRaises(ValueError, BinaryData().pack, "can't be string")
+        data= BinaryData('Some string')
+        self.assertRaises(ValueError, data.pack, "can't be string")