Merge pull request #55 from ottowayi/develop

Develop

README.rst

@@ -220,7 +220,6 @@ TODO
 
 - *(wip)* - improve documentation and include more real-world example scripts
 - *(not started)* - make API case insensitive
-- *(not started)* - support full structure writing
 
 
 License

pycomm3/__init__.py

@@ -24,7 +24,7 @@
 # SOFTWARE.
 #
 
-__version_info__ = (0, 7, 0)
+__version_info__ = (0, 7, 1)
 __version__ = '.'.join(f'{x}' for x in __version_info__)
 
 from typing import NamedTuple, Any, Optional

pycomm3/bytes_.py

@@ -79,7 +79,7 @@ def pack_ulong(l):
 
 
 def unpack_bool(st):
-    return not st[0] == 0
+    return st[0] != 0
 
 
 def unpack_sint(st):
@@ -165,8 +165,7 @@ def pack_char(char):
 
 
 def _short_string_encode(string):
-    encoded = pack_usint(len(string)) + b''.join([pack_char(x) for x in string])
-    return encoded
+    return pack_usint(len(string)) + b''.join([pack_char(x) for x in string])
 
 
 PACK_DATA_FUNCTION = {
@@ -190,8 +189,9 @@ def _short_string_encode(string):
 
 def _short_string_decode(str_data):
     string_len = str_data[0]
-    string = ''.join(chr(v + 256) if v < 0 else chr(v) for v in str_data[1:string_len+1])
-    return string
+    return ''.join(
+        chr(v + 256) if v < 0 else chr(v) for v in str_data[1:string_len + 1]
+    )
 
 
 UNPACK_DATA_FUNCTION = {

pycomm3/clx.py

@@ -350,8 +350,7 @@ def get_module_info(self, slot):
             response = request.send()
 
             if response:
-                info = _parse_identity_object(response.data)
-                return info
+                return _parse_identity_object(response.data)
             else:
                 raise DataError(f'send_rr_data did not return valid data - {response.error}')
 
@@ -365,20 +364,21 @@ def open(self):
         :return: True if successful, False otherwise
         """
         # handle the socket layer
-        if not self._connection_opened:
-            try:
-                if self._sock is None:
-                    self._sock = Socket()
-                self._sock.connect(self.attribs['ip address'], self.attribs['port'])
-                self._connection_opened = True
-                self.attribs['cid'] = urandom(4)
-                self.attribs['vsn'] = urandom(4)
-                if self._register_session() is None:
-                    self.__log.warning("Session not registered")
-                    return False
-                return True
-            except Exception as e:
-                raise CommError(e)
+        if self._connection_opened:
+            return
+        try:
+            if self._sock is None:
+                self._sock = Socket()
+            self._sock.connect(self.attribs['ip address'], self.attribs['port'])
+            self._connection_opened = True
+            self.attribs['cid'] = urandom(4)
+            self.attribs['vsn'] = urandom(4)
+            if self._register_session() is None:
+                self.__log.warning("Session not registered")
+                return False
+            return True
+        except Exception as e:
+            raise CommError(e)
 
     def _register_session(self) -> Optional[int]:
         """
@@ -935,7 +935,7 @@ def _parse_template_data(self, data, member_count):
         }
 
         for member, info in zip(member_names, member_data):
-            if not member.startswith('ZZZZZZZZZZ') and not member.startswith('__'):
+            if not (member.startswith('ZZZZZZZZZZ') or member.startswith('__')):
                 template['attributes'].append(member)
             template['internal_tags'][member] = info
 
@@ -1009,10 +1009,7 @@ def read(self, *tags: str) -> ReturnType:
                 else:
                     if result:
                         typ, bit = request_data['bit']
-                        if typ == 'bit':
-                            val = bool(result.value & (1 << bit))
-                        else:
-                            val = result.value[bit % 32]
+                        val = bool(result.value & 1 << bit) if typ == 'bit' else result.value[bit % 32]
                         results.append(Tag(tag, val, 'BOOL'))
                     else:
                         results.append(Tag(tag, None, None, result.error))
@@ -1273,18 +1270,13 @@ def _parse_tag_request(self, tag: str) -> Optional[Tuple[str, Optional[int], int
             if len(attrs) and attrs[-1].isdigit():
                 _bit = attrs.pop(-1)
                 bit = ('bit', int(_bit))
-                if not len(attrs):
-                    tag = base
-                else:
-                    tag = f"{base}.{''.join(attrs)}"
-
+                tag = base if not len(attrs) else f"{base}.{''.join(attrs)}"
             tag_info = self._get_tag_info(base, attrs)
 
             if tag_info['data_type'] == 'DWORD' and elements == 1:
                 _tag, idx = _get_array_index(tag)
                 tag = f'{_tag}[{idx // 32}]'
                 bit = ('bool_array', idx)
-                elements = 1
 
             return tag, bit, elements, tag_info
 
@@ -1398,8 +1390,7 @@ def _parse_plc_name(response):
         raise DataError(f'get_plc_name returned status {get_service_status(response.error)}')
     try:
         name_len = unpack_uint(response.data[6:8])
-        name = response.data[8: 8 + name_len].decode()
-        return name
+        return response.data[8: 8 + name_len].decode()
     except Exception as err:
         raise DataError(err)
 

pycomm3/packets/requests.py

@@ -500,15 +500,13 @@ def _make_write_data_bit(tag_info, value, request_path):
         raise RequestError(f'Invalid data type {tag_info["data_type"]} for writing bits')
 
     or_mask, and_mask = value
-    request_path = b''.join((
-        bytes([TAG_SERVICES_REQUEST["Read Modify Write Tag"]]),
-        request_path,
-        pack_uint(mask_size),
-        pack_udint(or_mask)[:mask_size],
-        pack_udint(and_mask)[:mask_size]
-    ))
-
-    return request_path
+    return b''.join((
+            bytes([TAG_SERVICES_REQUEST["Read Modify Write Tag"]]),
+            request_path,
+            pack_uint(mask_size),
+            pack_udint(or_mask)[:mask_size],
+            pack_udint(and_mask)[:mask_size]
+        ))
 
 
 @logged
@@ -558,43 +556,43 @@ def _create_tag_rp(tag, tag_cache, use_instance_ids):
     It returns the request packed wrapped around the tag passed.
     If any error it returns none
     """
+
     tags = tag.split('.')
     if tags:
         base, *attrs = tags
-
-        if use_instance_ids and base in tag_cache:
+        base_tag, index = _find_tag_index(base)
+        if use_instance_ids and base_tag in tag_cache:
             rp = [CLASS_TYPE['8-bit'],
                   CLASS_CODE['Symbol Object'],
                   INSTANCE_TYPE['16-bit'],
-                  pack_uint(tag_cache[base]['instance_id'])]
+                  pack_uint(tag_cache[base_tag]['instance_id'])]
         else:
-            base_tag, index = _find_tag_index(base)
             base_len = len(base_tag)
             rp = [EXTENDED_SYMBOL,
                   pack_usint(base_len),
-                  base_tag]
+                  base_tag.encode()]
             if base_len % 2:
                 rp.append(b'\x00')
-            if index is None:
-                return None
-            else:
-                rp += index
+        if index is None:
+            return None
+        else:
+            rp += _encode_tag_index(index)
 
         for attr in attrs:
             attr, index = _find_tag_index(attr)
             tag_length = len(attr)
             # Create the request path
             attr_path = [EXTENDED_SYMBOL,
                          pack_usint(tag_length),
-                         attr]
+                         attr.encode()]
             # Add pad byte because total length of Request path must be word-aligned
             if tag_length % 2:
                 attr_path.append(b'\x00')
             # Add any index
             if index is None:
                 return None
             else:
-                attr_path += index
+                attr_path += _encode_tag_index(index)
             rp += attr_path
 
         # At this point the Request Path is completed,
@@ -614,7 +612,7 @@ def _find_tag_index(tag):
         tag = t[:t.find('[')]  # Get only the tag part
     else:
         index = []
-    return tag.encode(), _encode_tag_index(index)
+    return tag, index
 
 
 def _encode_tag_index(index):

pycomm3/packets/responses.py

@@ -474,8 +474,7 @@ def parse_read_reply_struct(data, data_type):
 def parse_string(data):
     str_len = unpack_dint(data)
     str_data = data[4:4+str_len]
-    string = ''.join(chr(v + 256) if v < 0 else chr(v) for v in str_data)
-    return string
+    return ''.join(chr(v + 256) if v < 0 else chr(v) for v in str_data)
 
 
 def dword_to_bool_array(dword):