SonarQube - Reduce Complexity (#143)

* Refactor binary.py/_dict_to_protobuf()

* Replace RawMessage which is deprecated

* Refactor binary.py/_build_control_command_message

* Refactor binary.py/_protobuf_to_dict()

* Renamed methods for consistency

* Update method names to match previous renaming

* Refactor _build_control_command_parsed_message()

* Extract printing diagnostic response

* Remove unnecessary conditional statement

* Refactor control.py/main()

* Minimal refactor of scanner.py/scan()

* Refactor structures.py/merge_message()

* Refactor base.py/run()

Author: PPRAKA30

openxc/formats/binary.py

@@ -100,17 +100,7 @@ def id(self, value):
                 value = int(value, 0)
             self._id = value
 
-    def merge_message(self, data):
-        self.bus_name = self.bus_name or data.get('bus', None)
-
-        message_attributes = dir(self)
-        message_attributes = [a.replace('bus_name', 'bus') for a in message_attributes]
-        data_attributes = list(data.keys())
-        extra_attributes = set(data_attributes) - set(message_attributes)
-
-        if extra_attributes:
-            fatal_error('ERROR: Message %s has unrecognized attributes: %s' % (data.get('id'), ', '.join(extra_attributes)))
-
+    def validate_bus(self):
         if getattr(self, 'message_set'):
             self.bus = self.message_set.lookup_bus(name=self.bus_name)
             if not self.bus.valid():
@@ -123,6 +113,19 @@ def merge_message(self, data):
                     msg = "Bus '%s' is disabled" % self.bus_name
                 LOG.warning("%s - message 0x%x will be disabled" % (msg, self.id))
 
+    def merge_message(self, data):
+        self.bus_name = self.bus_name or data.get('bus', None)
+
+        message_attributes = dir(self)
+        message_attributes = [a.replace('bus_name', 'bus') for a in message_attributes]
+        data_attributes = list(data.keys())
+        extra_attributes = set(data_attributes) - set(message_attributes)
+
+        if extra_attributes:
+            fatal_error('ERROR: Message %s has unrecognized attributes: %s' % (data.get('id'), ', '.join(extra_attributes)))
+
+        self.validate_bus()
+
         self.id = self.id or data.get('id')
         self.name = self.name or data.get('name', None)
         self.bit_numbering_inverted = (self.bit_numbering_inverted or

openxc/generator/structures.py

@@ -185,6 +185,20 @@ def _message_valid(self, message):
             return False
         return True
 
+    def parse_messages(self):
+        while True:
+            message = self.streamer.parse_next_message()
+            if message is None:
+                break
+
+            if not self._message_valid(message):
+                self.corrupted_messages += 1
+                break
+
+            if self.callback is not None:
+                self.callback(message)
+            self._receive_command_response(message)
+
     def run(self):
         """Continuously read data from the source and attempt to parse a valid
         message from the buffer of bytes. When a message is parsed, passes it
@@ -210,18 +224,7 @@ def run(self):
                     self.format = "protobuf"
             self.streamer.receive(payload)
 
-            while True:
-                message = self.streamer.parse_next_message()
-                if message is None:
-                    break
-
-                if not self._message_valid(message):
-                    self.corrupted_messages += 1
-                    break
-
-                if self.callback is not None:
-                    self.callback(message)
-                self._receive_command_response(message)
+            self.parse_messages()
 
     def _receive_command_response(self, message):
         # TODO the controller/source are getting a little mixed up since the

openxc/sources/base.py

@@ -134,6 +134,37 @@ def parse_options():
     parser.set_defaults(format="json")
     return parser.parse_args()
 
+def handle_set_command(arguments, interface):
+    if arguments.passthrough_enabled is not None:
+        passthrough(interface, int(arguments.bus), arguments.passthrough_enabled)
+    if arguments.af_bypass is not None:
+        af_bypass(interface, int(arguments.bus), arguments.af_bypass)
+    if arguments.new_payload_format is not None:
+        set_payload_format(interface, arguments.new_payload_format)
+    if arguments.unix_time is not None:
+        set_rtc_time(interface, int(arguments.unix_time))
+    if arguments.host is not None:
+        modem_configuration(interface, arguments.host, arguments.port)
+
+def handle_write_command(arguments, interface):
+    if arguments.write_name:
+        interface.write(name=arguments.write_name,
+                value=arguments.write_value,
+                event=arguments.write_event)
+    elif arguments.write_id:
+        if not arguments.write_data:
+            sys.exit("%s requires an id and data" % arguments.command)
+        # TODO we should use unhexlify as with the diagnostic command
+        # payloads so we can standardize the API and not deal with hex
+        # strings in code
+        interface.write(bus=int(arguments.bus),
+                id=arguments.write_id,
+                data=arguments.write_data,
+                frame_format=arguments.write_frame_format)
+    elif arguments.write_input_file:
+        write_file(interface, arguments.write_input_file)
+    else:
+        sys.exit("%s requires a signal name, message ID or filename" % arguments.command)
 
 def main():
     configure_logging()
@@ -152,35 +183,8 @@ def main():
     elif arguments.command == "id":
         device_id(interface)
     elif arguments.command == "set":
-        if arguments.passthrough_enabled is not None:
-            passthrough(interface, int(arguments.bus), arguments.passthrough_enabled)
-        if arguments.af_bypass is not None:
-            af_bypass(interface, int(arguments.bus), arguments.af_bypass)
-        if arguments.new_payload_format is not None:
-            set_payload_format(interface, arguments.new_payload_format)
-        if arguments.unix_time is not None:
-            set_rtc_time(interface, int(arguments.unix_time))
-        if arguments.host is not None:
-            modem_configuration(interface, arguments.host, arguments.port)
-    elif arguments.command.startswith("write"):
-        if arguments.command == "write":
-            if arguments.write_name:
-                interface.write(name=arguments.write_name,
-                        value=arguments.write_value,
-                        event=arguments.write_event)
-            elif arguments.write_id:
-                if not arguments.write_data:
-                    sys.exit("%s requires an id and data" % arguments.command)
-                # TODO we should use unhexlify as with the diagnostic command
-                # payloads so we can standardize the API and not deal with hex
-                # strings in code
-                interface.write(bus=int(arguments.bus),
-                        id=arguments.write_id,
-                        data=arguments.write_data,
-                        frame_format=arguments.write_frame_format)
-            elif arguments.write_input_file:
-                write_file(interface, arguments.write_input_file)
-            else:
-                sys.exit("%s requires a signal name, message ID or filename" % arguments.command)
+        handle_set_command(arguments, interface)
+    elif arguments.command == "write":
+        handle_write_command(arguments, interface)
     else:
         print(("Unrecognized command \"%s\"" % arguments.command))

openxc/tools/control.py

@@ -13,6 +13,23 @@
 
 from .common import device_options, configure_logging, select_device
 
+def print_diagnostic_response(responses):
+    for response in responses:
+        # After sending a diagnostic request, it will return with a signal message saying if the
+        # request was recieved. After that it used to show about 30 vehicle messages (rpm, speed, etc)
+        # with the actual diagnostic response mixed in. So, if the response length is more than
+        # 1, it's the response, if its less (only 1) it's the recieved message.
+        if len(response) > 1:
+            # Stripping all of the unnesseary data we get after sending a diag request in python
+            # Just like in enabler, it's a diag response if it contains the keys "mode", "bus", 
+            # "id", and "success". 
+            diag_mgs_req_keys = ['mode', 'bus', 'id', 'success']
+            indices = [i for i, s in enumerate(response) if all(x in s for x in diag_mgs_req_keys)]
+            if indices:
+                print(("Response: %s" % response[indices[0]]))
+        else:
+            print(("Response: %s" % response))
+
 def diagnostic_request(arguments, controller):
     message = int(arguments.message, 0)
     mode = int(arguments.mode, 0)
@@ -40,21 +57,7 @@ def diagnostic_request(arguments, controller):
         if len(responses) == 0:
             print("No response received before timeout")
         else: 
-            for response in responses:
-                # After sending a diagnostic request, it will return with a signal message saying if the
-                # request was recieved. After that it used to show about 30 vehicle messages (rpm, speed, etc)
-                # with the actual diagnostic response mixed in. So, if the response length is more than
-                # 1, it's the response, if its less (only 1) it's the recieved message.
-                if len(response) > 1:
-                    # Stripping all of the unnesseary data we get after sending a diag request in python
-                    # Just like in enabler, it's a diag response if it contains the keys "mode", "bus", 
-                    # "id", and "success". 
-                    diag_mgs_req_keys = ['mode', 'bus', 'id', 'success']
-                    indices = [i for i, s in enumerate(response) if all(x in s for x in diag_mgs_req_keys)]
-                    if indices:
-                        print(("Response: %s" % response[indices[0]]))
-                else:
-                    print(("Response: %s" % response))
+            print_diagnostic_response(responses)
     elif arguments.command == "cancel":
         if controller.delete_diagnostic_request(message, mode, bus=bus,
                 pid=pid):

openxc/tools/diagnostics.py

@@ -15,15 +15,7 @@
 TESTER_PRESENT_MODE = 0x3e
 TESTER_PRESENT_PAYLOAD = bytearray([0])
 
-def scan(controller, bus=None, message_id=None):
-    message_ids = []
-    if message_id is not None:
-        message_ids.append(message_id)
-    else:
-        # using 11-bit IDs
-        message_ids = list(range(0, 0x7ff + 1))
-
-    print("Sending tester present message to find valid modules arb IDs")
+def find_active_modules(controller, bus, message_ids):
     active_modules = set()
     for arb_id in message_ids:
         response = controller.create_diagnostic_request(arb_id, TESTER_PRESENT_MODE,
@@ -32,9 +24,9 @@ def scan(controller, bus=None, message_id=None):
         if response is not None:
             print(("0x%x responded to tester present: %s" % (arb_id, response)))
             active_modules.add(arb_id)
+    return active_modules
 
-    # Scan for active services on each active module by sending blank requests
-    print("Scanning for services on active modules")
+def find_active_modes(controller, bus, active_modules):
     active_modes = defaultdict(list)
     for active_module in active_modules:
         controller.create_diagnostic_request(active_module, TESTER_PRESENT_MODE,
@@ -56,6 +48,22 @@ def scan(controller, bus=None, message_id=None):
 
         controller.create_diagnostic_request(active_module, TESTER_PRESENT_MODE, bus=bus,
                 frequency=0)
+    return active_modes
+
+def scan(controller, bus=None, message_id=None):
+    message_ids = []
+    if message_id is not None:
+        message_ids.append(message_id)
+    else:
+        # using 11-bit IDs
+        message_ids = list(range(0, 0x7ff + 1))
+
+    print("Sending tester present message to find valid modules arb IDs")
+    active_modules = find_active_modules(controller, bus, message_ids)
+
+    # Scan for active services on each active module by sending blank requests
+    print("Scanning for services on active modules")
+    active_modes = find_active_modes(controller, bus, active_modules)
 
     # Scan for what each mode can do and what data it can return by fuzzing the
     # payloads