Change BinaryData.pack method

- Fixed test
- Removed _pack method
- BinaryData.pack accept only packable values, None and bytes

Author: macartur

pyof/foundation/basic_types.py

@@ -428,46 +428,35 @@ class BinaryData(GenericType):
     return the size of the instance using Python's :func:`len`.
     """
 
-    def __init__(self, value=None):  # 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.
-
         """
-        value = self._pack(value)
         super().__init__(value)
 
-    @staticmethod
-    def _pack(value):
-        if hasattr(value, 'pack') and callable(value.pack):
-            value = value.pack()
-        elif value is None:
-            value = b''
-
-        if not isinstance(value, bytes):
-            msg = 'BinaryData value must contain bytes or have pack method. '
-            msg += 'Received type {} value: "{}"'.format(type(value), value)
-            raise ValueError(msg)
-
-        return value
-
     def pack(self, value=None):
         """Pack the value as a binary representation.
 
         Returns:
             bytes: The binary representation.
 
         Raises:
-            :exc:`~.exceptions.NotBinaryData`: If value is not :class:`bytes`.
+            ValueError: If value can't be represented with bytes
 
         """
         if value is None:
-            return self._value
-        return self._pack(value)
+            value = self._value
+
+        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.
@@ -494,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,13 +172,13 @@ def test_default_value(self):
         actual = BinaryData().pack()
         self.assertEqual(expected, actual)
 
-    def test_none_value(self):
-        """Pack of BinaryData initialized with None should be empty bytes."""
+    def test_pack_none_value(self):
+        """Test packing None value."""
         expected = b''
         actual = BinaryData(None).pack()
         self.assertEqual(expected, actual)
 
-    def test_pack_bytes(self):
+    def test_pack_bytes_value(self):
         """Test packing some bytes."""
         expected = b'forty two'
         actual = BinaryData(expected).pack()
@@ -190,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")