diff --git a/src/fixgw/plugins/canfix/__init__.py b/src/fixgw/plugins/canfix/__init__.py index fa833ad8..7d46faf3 100644 --- a/src/fixgw/plugins/canfix/__init__.py +++ b/src/fixgw/plugins/canfix/__init__.py @@ -25,6 +25,7 @@ import canfix from . import mapping +import fixgw.quorum as quorum class MainThread(threading.Thread): @@ -80,17 +81,19 @@ def run(self): data.append(0x00) # Function codes data.extend(msg.data[3:]) msg.data = data + else: + self.parent.recvignorecount += 1 + continue except: - pass - if ( - self.parent.quorum.enabled - and msg.data[0] == 6 - and msg.data[1] == 9 - ) and (msg.arbitration_id > 1759 and msg.arbitration_id < 2016): + self.parent.recvinvalidcount += 1 + continue + if (quorum.enabled and msg.data[0] == 6 and msg.data[1] == 9) and ( + msg.arbitration_id > 1759 and msg.arbitration_id < 2016 + ): # This is a quorum node status message # We only want ones that are not our own cfobj = canfix.parseMessage(msg) - if cfobj.value != self.parent.quorum.nodeid: + if cfobj.value != quorum.nodeid: # This is not ourself if cfobj.value > 0 and cfobj.value < 100: try: @@ -98,27 +101,38 @@ def run(self): f"QVOTE{cfobj.value}", cfobj.value ) except: + self.parent.recvinvalidcount += 1 self.log.warning( f"Received a vote for QVOTE{cfobj.value} but this fixid does not exist" ) + else: + self.parent.recvinvalidcount += 1 + else: + # We ignore out own messages + self.parent.recvignorecount += 1 + continue if self.interesting[msg.arbitration_id]: try: cfobj = canfix.parseMessage(msg) except ValueError as e: + self.parent.recvinvalidcount += 1 self.log.warning(e) else: self.log.debug( # A bug in canfix lib __str__ causes exception if indexName is not defined # So changed this to output cfobj.getName instead of cfobj # when canfix bug is resolved should change this back - "Fix Thread parseFrame() returned, {0}".format(cfobj.getName) + "Fix Thread parseFrame() returned, {0}".format( + cfobj.getName + ) ) if isinstance(cfobj, canfix.Parameter): self.mapping.inputMap(cfobj) else: # TODO What to do with the other types - pass - # # TODO increment error counter + self.parent.recvignorecount += 1 + else: + self.parent.recvignorecount += 1 finally: if self.getout: break @@ -138,7 +152,12 @@ def __init__(self, name, config): self.mapping = mapping.Mapping(mapfilename, self.log) self.thread = MainThread(self, config) self.recvcount = 0 - self.errorcount = 0 + self.recvignorecount = 0 + self.recvinvalidcount = 0 + self.mapping.sendcount = 0 + self.mapping.senderrorcount = 0 + self.mapping.recvignorecount = 0 + self.mapping.recvinvalidcount = 0 def run(self): self.bus = can.ThreadSafeBus(self.channel, interface=self.interface) @@ -146,7 +165,7 @@ def run(self): self.db_callback_add( each, self.mapping.getOutputFunction(self.bus, each, self.node) ) - if self.quorum.enabled: + if quorum.enabled: # canfix needs updated to support quorum so we will monkey patch it for now # Pull to fix canfix: https://github.com/birkelbach/python-canfix/pull/13 # Request to add this to the canfix specification: https://github.com/makerplane/canfix-spec/issues/4 @@ -164,9 +183,9 @@ def run(self): ) # Added callback to transmit our quorum vote on the bus self.db_callback_add( - self.quorum.vote_key, + quorum.vote_key, self.mapping.getQuorumOutputFunction( - self.bus, self.quorum.vote_key, self.node + self.bus, quorum.vote_key, self.node ), ) self.thread.start() @@ -186,8 +205,10 @@ def get_status(self): x["CAN Interface"] = self.interface x["CAN Channel"] = self.channel x["Received Frames"] = self.recvcount + x["Ignored Frames"] = self.recvignorecount + self.mapping.recvignorecount + x["Invalid Frames"] = self.recvinvalidcount + self.mapping.recvinvalidcount x["Sent Frames"] = self.mapping.sendcount - x["Error Count"] = self.errorcount + x["Send Error Count"] = self.mapping.senderrorcount return x diff --git a/src/fixgw/plugins/canfix/mapping.py b/src/fixgw/plugins/canfix/mapping.py index 51483b88..4ea83150 100644 --- a/src/fixgw/plugins/canfix/mapping.py +++ b/src/fixgw/plugins/canfix/mapping.py @@ -36,6 +36,9 @@ def __init__(self, mapfile, log=None): self.output_mapping = {} self.log = log self.sendcount = 0 + self.senderrorcount = 0 + self.recvignorecount = 0 + self.recvinvalidcount = 0 # Open and parse the YAML mapping file passed to us try: @@ -144,6 +147,7 @@ def InputFunc(cfpar): m = cfpar.meta dbItem.set_aux_value(m, cfpar.value) except: + self.recvinvalidcount += 1 self.log.warning( "Problem setting Aux Value for {0}".format(dbItem.key) ) @@ -156,8 +160,7 @@ def InputFunc(cfpar): cfpar.failure, ) else: - # WTF to do here? - pass + self.recvinvalidcount += 1 return InputFunc # Returns a closure that should be used as the callback for database item @@ -238,6 +241,7 @@ def outputCallback(key, value, udata): try: bus.send(p.msg) except Exception as e: + self.senderrorcount += 1 self.log.error("CAN send failure:" + str(e)) # This does not seem to always flush the buffer # a full tx queue seems to be the most common error @@ -269,13 +273,16 @@ def outputCallback(key, value, udata): try: bus.send(p.msg) except Exception as e: + self.senderrorcount += 1 + self.log.debug(f"Output {dbKey}: Send Failed {p.msg}") self.log.error("CAN send failure:" + str(e)) # This does not seem to always flush the buffer # a full tx queue seems to be the most common error # when the bus is disrupted bus.flush_tx_buffer() - self.sendcount += 1 - self.log.debug(f"Output {dbKey}: Sent {p.msg}") + else: + self.sendcount += 1 + self.log.debug(f"Output {dbKey}: Sent {p.msg}") return outputCallback @@ -290,12 +297,15 @@ def outputCallback(key, value, udata): try: bus.send(p.msg) except Exception as e: + self.senderrorcount += 1 + self.log.debug(f"Output {dbKey}: Send Failed {p.msg}") self.log.error("CAN send failure:" + str(e)) # This does not seem to always flush the buffer # a full tx queue seems to be the most common error # when the bus is disrupted bus.flush_tx_buffer() - self.sendcount += 1 + else: + self.sendcount += 1 return outputCallback diff --git a/tests/test_canfix.py b/tests/test_canfix.py index 7beec0ef..09fa9029 100644 --- a/tests/test_canfix.py +++ b/tests/test_canfix.py @@ -208,6 +208,8 @@ def plugin(): ) pl.shutdown() can_bus.shutdown() + quorum.enabled = False + quorum.nodeid = None def test_missing_mapfile(): @@ -230,12 +232,25 @@ def test_parameter_writes(plugin): val = x[0] assert abs(val - param[3]) <= param[4] + status = plugin.pl.get_status() + assert status["Received Frames"] == len(ptests) + assert status["Ignored Frames"] == 0 + assert status["Invalid Frames"] == 0 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 + def test_unowned_outputs(plugin): database.write("BARO", 30.04) msg = plugin.bus.recv(1.0) assert msg.arbitration_id == plugin.node + 1760 assert msg.data == bytearray([12, 0x90, 0x01, 0x58, 0x75]) + status = plugin.pl.get_status() + assert status["Received Frames"] == 0 + assert status["Ignored Frames"] == 0 + assert status["Invalid Frames"] == 0 + assert status["Sent Frames"] == 1 + assert status["Send Error Count"] == 0 def test_all_frame_ids(plugin): @@ -246,6 +261,7 @@ def test_all_frame_ids(plugin): # Check against venv/lib/python3.10/site-packages/canfix/protocol.py parameters[pid] import canfix.protocol + count = 0 for id in range(2048): if id in canfix.protocol.parameters: for dsize in range(9): @@ -254,13 +270,20 @@ def test_all_frame_ids(plugin): msg.data.append(random.randrange(256)) msg.dlc = dsize plugin.bus.send(msg) + count += 1 + time.sleep(0.03) + status = plugin.pl.get_status() + assert status["Received Frames"] == count + assert status["Ignored Frames"] == 3294 + assert status["Invalid Frames"] == 10 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 # TODO Add asserts above -def test_quorum_outputs_diabled(plugin): - quorum.enabled = False +def test_ignore_quorum_mssages_when_diabled(plugin): for param in qtests: p = canfix.NodeStatus() p.sendNode = param[1] @@ -269,46 +292,83 @@ def test_quorum_outputs_diabled(plugin): plugin.bus.send(p.msg) time.sleep(0.03) x = database.read(param[0]) + # Nothing should change since we are not accepting the messages assert x[0] == 0 + status = plugin.pl.get_status() + # All received frames should be ignored + assert status["Received Frames"] == len(qtests) + assert status["Ignored Frames"] == len(qtests) + assert status["Invalid Frames"] == 0 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 -def test_quorum_outputs_enabled(plugin, caplog): +def test_accept_quorum_mssages_when_enabled(plugin): quorum.enabled = True quorum.nodeid = 1 p = canfix.NodeStatus() + # keep track of the frames we should ignore + ignoreframes = 0 for param in qtests: + if param[1] == quorum.nodeid: + ignoreframes += 1 p.sendNode = param[1] p.parameter = 0x09 p.value = param[2] plugin.bus.send(p.msg) time.sleep(0.03) x = database.read(param[0]) - # print(f"{param[0]}: {x[0]}, {param[1]} {param[2]}") if param[2] == 1: assert x[0] == 0 else: assert x[0] == param[2] - p.sendNode = 5 + status = plugin.pl.get_status() + assert status["Received Frames"] == len(qtests) + assert status["Ignored Frames"] == ignoreframes + assert status["Invalid Frames"] == 0 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 + + +def test_reject_invalid_quorum_mssages_when_enabled(plugin, caplog): + quorum.enabled = True + quorum.nodeid = 1 + p = canfix.NodeStatus() + p.parameter = 0x09 + # keep track of the frames we should ignore + sentframes = 0 + invalidframes = 0 # Test invalid value 101 + p.sendNode = 5 p.value = 101 plugin.bus.send(p.msg) time.sleep(0.03) + invalidframes += 1 + sentframes += 1 assert database.read("QVOTE5")[0] == 0 # Test invalid value 0 p.value = 0 plugin.bus.send(p.msg) time.sleep(0.03) + invalidframes += 1 + sentframes += 1 assert database.read("QVOTE5")[0] == 0 + # Test DB not configured for 6 nodes p.sendNode = 6 p.value = 6 + invalidframes += 1 + sentframes += 1 with caplog.at_level(logging.WARNING): plugin.bus.send(p.msg) time.sleep(0.03) assert "Received a vote for QVOTE6 but this fixid does not exist" in caplog.text - - quorum.enabled = False - quorum.nodeid = None + status = plugin.pl.get_status() + assert status["Received Frames"] == sentframes + assert status["Ignored Frames"] == 0 + assert status["Invalid Frames"] == invalidframes + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 def test_switch_inputs(plugin): @@ -350,6 +410,12 @@ def test_switch_inputs(plugin): time.sleep(0.03) # Sending false for toggle does not change state assert database.read("TSBTN124")[0] == False + status = plugin.pl.get_status() + assert status["Received Frames"] == 6 + assert status["Ignored Frames"] == 0 + assert status["Invalid Frames"] == 0 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 def test_nodespecific_switch_inputs(plugin): @@ -387,6 +453,12 @@ def test_nodespecific_switch_inputs(plugin): assert database.read("MAVWPVALID")[0] == False assert database.read("MAVREQADJ")[0] == False assert database.read("MAVREQAUTOTUNE")[0] == False + status = plugin.pl.get_status() + assert status["Received Frames"] == 2 + assert status["Ignored Frames"] == 0 + assert status["Invalid Frames"] == 0 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 def test_nodespecific_switch_input_that_we_do_not_want(plugin): @@ -413,10 +485,16 @@ def test_nodespecific_switch_input_that_we_do_not_want(plugin): time.sleep(0.03) # Since we do not need either of these messages they are never parsed mock_parse.assert_not_called() + status = plugin.pl.get_status() + assert status["Received Frames"] == 2 + assert status["Ignored Frames"] == 1 + assert status["Invalid Frames"] == 1 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 def test_bad_parse(plugin): - # Bad CAN data can cause exceptions if the code does not + # Bad CAN data can cause exceptions if the code does not # Ensure the data is valid before using it # I found, and fixed, such a bug when writing a test # This test ensures we do not have regressions @@ -432,6 +510,13 @@ def test_bad_parse(plugin): pytest.fail(f"An unexpected exception occurred: {e}") # Data should not change if bad data is sent assert cur_vs == database.read("VS")[0] + status = plugin.pl.get_status() + assert status["Received Frames"] == 1 + assert status["Ignored Frames"] == 0 + assert status["Invalid Frames"] == 1 + assert status["Sent Frames"] == 0 + assert status["Send Error Count"] == 0 + def test_get_status(plugin): status = plugin.pl.get_status()