From 090ba3042c781d19bd06505dc020a81e9e06aacd Mon Sep 17 00:00:00 2001 From: Brian ONeill Date: Wed, 5 May 2021 03:50:33 -0700 Subject: [PATCH 1/7] Fix issue where last item in OD file may not be parsed --- canopen_monitor/parse/eds.py | 2 +- tests/spec_eds_parser.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/canopen_monitor/parse/eds.py b/canopen_monitor/parse/eds.py index 933c3a3..2a25858 100644 --- a/canopen_monitor/parse/eds.py +++ b/canopen_monitor/parse/eds.py @@ -144,7 +144,7 @@ def __init__(self, eds_data: [str]): prev = 0 for i, line in enumerate(eds_data): - if line == '': + if line == '' or i == len(eds_data) - 1: # Handle extra empty strings if prev == i: prev = i + 1 diff --git a/tests/spec_eds_parser.py b/tests/spec_eds_parser.py index 6b7e055..eeb5a9c 100644 --- a/tests/spec_eds_parser.py +++ b/tests/spec_eds_parser.py @@ -70,6 +70,11 @@ def test_named_sections(self): self.eds.mandatory_objects.supported_objects, "Error parsing Comments named section") + def test_last_index(self): + self.assertEqual("Last Aolved filepath", + self.eds[hex(0x3102)].parameter_name, + "Error parsing last index location") + class TestDCF(unittest.TestCase): def setUp(self): From c506ee258166e35af19132af588a7c62d061f35f Mon Sep 17 00:00:00 2001 From: Brian ONeill Date: Wed, 5 May 2021 09:24:40 -0700 Subject: [PATCH 2/7] Tests added and version incremented --- canopen_monitor/__init__.py | 2 +- tests/spec_eds_parser.py | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/canopen_monitor/__init__.py b/canopen_monitor/__init__.py index acaa3b2..90fe29a 100755 --- a/canopen_monitor/__init__.py +++ b/canopen_monitor/__init__.py @@ -2,7 +2,7 @@ MAJOR = 3 MINOR = 3 -PATCH = 1 +PATCH = 2 APP_NAME = 'canopen-monitor' APP_DESCRIPTION = 'An NCurses-based TUI application for tracking activity' \ diff --git a/tests/spec_eds_parser.py b/tests/spec_eds_parser.py index eeb5a9c..19b155a 100644 --- a/tests/spec_eds_parser.py +++ b/tests/spec_eds_parser.py @@ -71,6 +71,15 @@ def test_named_sections(self): "Error parsing Comments named section") def test_last_index(self): + """ + Parsing should capture the last index if there is no newline + """ + file_check = TEST_EDS.splitlines() + self.assertEqual("PDOMapping=0", + file_check[len(file_check)-1], + "The last line in the EDS test file should not be " + "blank") + self.assertEqual("Last Aolved filepath", self.eds[hex(0x3102)].parameter_name, "Error parsing last index location") @@ -96,3 +105,21 @@ def test_get_node_id(self): self.assertEqual(10, self.eds.node_id, "Error parsing node id") + + def test_existing_spaces(self): + """ + DCF tests should test importing a file with arbitrary blank lines + This test confirms that the test file contains those blank lines + """ + file_check = TEST_DCF.splitlines() + self.assertEqual("", + file_check[len(file_check) - 1], + "The last line in the DCF Test file should be blank") + + self.assertEqual("", + file_check[len(file_check) - 12], + "There should be 2 blank lines before the last index") + + self.assertEqual("", + file_check[len(file_check) - 13], + "There should be 2 blank lines before the last index") From b54b9865776ab43b4422f12a01b6c6210753e1b2 Mon Sep 17 00:00:00 2001 From: dmitri-mcguckin Date: Sun, 9 May 2021 19:23:58 -0400 Subject: [PATCH 3/7] Fix parse node name access in message table --- canopen_monitor/app.py | 4 ++-- canopen_monitor/assets/dcfs/battery.dcf | 1 - canopen_monitor/can/message.py | 2 +- canopen_monitor/can/message_table.py | 3 ++- canopen_monitor/parse/canopen.py | 9 ++++++++- canopen_monitor/ui/message_pane.py | 2 +- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/canopen_monitor/app.py b/canopen_monitor/app.py index cd07631..e5ecf2b 100644 --- a/canopen_monitor/app.py +++ b/canopen_monitor/app.py @@ -175,7 +175,7 @@ def __enter__(self: App) -> App: header='Remove Interface', footer='ENTER: remove, F5: exit window', style=curses.color_pair(1)) - self.hb_pane = MessagePane(cols={'Node ID': ('node_name', 0, hex), + self.hb_pane = MessagePane(cols={'Node ID': ('node_name', 0), 'State': ('state', 0), 'Status': ('message', 0)}, types=[MessageType.HEARTBEAT], @@ -187,7 +187,7 @@ def __enter__(self: App) -> App: name='Heartbeats', message_table=self.table) self.misc_pane = MessagePane(cols={'COB ID': ('arb_id', 0, pad_hex), - 'Node Name': ('node_name', 0, hex), + 'Node Name': ('node_name', 0), 'Type': ('type', 0), 'Message': ('message', 0)}, types=[MessageType.NMT, diff --git a/canopen_monitor/assets/dcfs/battery.dcf b/canopen_monitor/assets/dcfs/battery.dcf index 264d8c1..e262e33 100644 --- a/canopen_monitor/assets/dcfs/battery.dcf +++ b/canopen_monitor/assets/dcfs/battery.dcf @@ -7233,4 +7233,3 @@ AccessType=ro DefaultValue=00000000000000000000 ParameterValue= PDOMapping=0 - diff --git a/canopen_monitor/can/message.py b/canopen_monitor/can/message.py index d20ef7e..2800f55 100644 --- a/canopen_monitor/can/message.py +++ b/canopen_monitor/can/message.py @@ -141,7 +141,7 @@ class Message(Frame): def __init__(self: Message, arb_id: int, **kwargs): super().__init__(arb_id, **kwargs) - self.node_name = MessageType.cob_to_node(self.type, self.arb_id) + self.node_name = 'N/A' self.message = self.data @property diff --git a/canopen_monitor/can/message_table.py b/canopen_monitor/can/message_table.py index 4794460..7884b4c 100644 --- a/canopen_monitor/can/message_table.py +++ b/canopen_monitor/can/message_table.py @@ -4,11 +4,12 @@ class MessageTable: def __init__(self: MessageTable, parser=None): - self.parser = parser self.table = {} + self.parser = parser def __add__(self: MessageTable, message: Message) -> MessageTable: if(self.parser is not None): + message.node_name = self.parser.get_name(message) message.message = self.parser.parse(message) self.table[message.arb_id] = message return self diff --git a/canopen_monitor/parse/canopen.py b/canopen_monitor/parse/canopen.py index 30561e6..31b2dbc 100644 --- a/canopen_monitor/parse/canopen.py +++ b/canopen_monitor/parse/canopen.py @@ -1,3 +1,4 @@ +from typing import Union from ..can import Message, MessageType from . import hb as HBParser, \ pdo as PDOParser, \ @@ -16,6 +17,12 @@ def __init__(self, eds_configs: dict): self.sdo_parser = SDOParser() self.eds_configs = eds_configs + def get_name(self, message: Message) -> Union[str, None]: + # import ipdb; ipdb.set_trace() + parser = self.eds_configs.get(message.node_id) + return parser.device_commissioning.node_name \ + if parser else hex(message.node_id) + def parse(self, message: Message) -> str: """ Detect the type of the given message and return the parsed version @@ -30,7 +37,7 @@ def parse(self, message: Message) -> str: """ node_id = message.node_id - eds_config = self.eds_configs.get(hex(node_id)) \ + eds_config = self.eds_configs.get(node_id) \ if node_id is not None else None # Detect message type and select the appropriate parse function diff --git a/canopen_monitor/ui/message_pane.py b/canopen_monitor/ui/message_pane.py index 1ced972..b364c99 100644 --- a/canopen_monitor/ui/message_pane.py +++ b/canopen_monitor/ui/message_pane.py @@ -213,7 +213,7 @@ def draw(self: MessagePane) -> None: def __reset_col_widths(self: Message): """ - Reset the width of Pane collumn. + Reset the width of Pane collumn. Based on the length of data to change the width. """ for name, data in self.cols.items(): From 38ac9f5b877ab3610b5c1a1cf29033dfc14740ef Mon Sep 17 00:00:00 2001 From: dmitri-mcguckin Date: Sun, 9 May 2021 20:03:07 -0400 Subject: [PATCH 4/7] Add boilerplate Meta config class --- canopen_monitor/app.py | 1 + canopen_monitor/can/message.py | 4 ++-- canopen_monitor/meta.py | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 canopen_monitor/meta.py diff --git a/canopen_monitor/app.py b/canopen_monitor/app.py index e5ecf2b..77563ea 100644 --- a/canopen_monitor/app.py +++ b/canopen_monitor/app.py @@ -189,6 +189,7 @@ def __enter__(self: App) -> App: self.misc_pane = MessagePane(cols={'COB ID': ('arb_id', 0, pad_hex), 'Node Name': ('node_name', 0), 'Type': ('type', 0), + 'Age': ('age', 0), 'Message': ('message', 0)}, types=[MessageType.NMT, MessageType.SYNC, diff --git a/canopen_monitor/can/message.py b/canopen_monitor/can/message.py index 2800f55..bafdffc 100644 --- a/canopen_monitor/can/message.py +++ b/canopen_monitor/can/message.py @@ -3,8 +3,8 @@ from enum import Enum from pyvit.can import Frame -STALE_TIME = dt.timedelta(minutes=2) -DEAD_TIME = dt.timedelta(minutes=4) +STALE_TIME = dt.timedelta(seconds=5) +DEAD_TIME = dt.timedelta(seconds=10) class MessageType(Enum): diff --git a/canopen_monitor/meta.py b/canopen_monitor/meta.py new file mode 100644 index 0000000..9c5dc2d --- /dev/null +++ b/canopen_monitor/meta.py @@ -0,0 +1,18 @@ +from .can import MagicCANBus + + +class Meta: + def __init__(self, config_dir, cache_dir): + self.config_dir = config_dir + self.cache_dir = cache_dir + + def save_devices(self, mcb: MagicCANBus) -> bool: + # TODO: Save the list of devices in MCB + pass + + def load_devices(self) -> [str]: + # TODO: Pass back list of interface names to populate MCB + pass + + def load_node_overrides(self) -> dict: + pass From 144ede4042a74e9ce81fea0663b7f7b55fa61cda Mon Sep 17 00:00:00 2001 From: Brian ONeill Date: Mon, 10 May 2021 12:47:34 -0700 Subject: [PATCH 5/7] Add meta object and interface configuration saving --- canopen_monitor/__main__.py | 21 ++---- canopen_monitor/app.py | 6 +- canopen_monitor/meta.py | 24 +++++-- tests/spec_meta.py | 128 ++++++++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 22 deletions(-) create mode 100644 tests/spec_meta.py diff --git a/canopen_monitor/__main__.py b/canopen_monitor/__main__.py index 10f2c5c..899daa8 100755 --- a/canopen_monitor/__main__.py +++ b/canopen_monitor/__main__.py @@ -3,6 +3,7 @@ import argparse from . import APP_NAME, APP_VERSION, APP_DESCRIPTION, CONFIG_DIR, CACHE_DIR from .app import App +from .meta import Meta from .can import MagicCANBus, MessageTable from .parse import CANOpenParser, load_eds_file @@ -50,27 +51,15 @@ def main(): sys.exit(0) try: - if(len(args.interfaces) == 0): - print('Warning: no interfaces config was found and you did not' - ' specify any interface arguments') - print(f'\t(see {APP_NAME} -h for details)\n') - print('This means the monitor will not be listening to anything.') - while(True): - answer = input('Would you like to continue anyways? [y/N]: ') - if(answer.upper() == 'N' or answer == ''): - sys.exit(0) - elif(answer.upper() == 'Y'): - break - else: - print(f'Invalid response: {answer}') - init_dirs() eds_configs = load_eds_files() mt = MessageTable(CANOpenParser(eds_configs)) + meta = Meta(CONFIG_DIR, CACHE_DIR) + interfaces = meta.load_devices(args.interfaces) # Start the can bus and the curses app - with MagicCANBus(args.interfaces, no_block=args.no_block) as bus, \ - App(mt, eds_configs, bus) as app: + with MagicCANBus(interfaces, no_block=args.no_block) as bus, \ + App(mt, eds_configs, bus, meta) as app: while True: # Bus updates for message in bus: diff --git a/canopen_monitor/app.py b/canopen_monitor/app.py index 77563ea..24c25fd 100644 --- a/canopen_monitor/app.py +++ b/canopen_monitor/app.py @@ -10,6 +10,7 @@ from .can import MessageTable, MessageType, MagicCANBus from .ui import MessagePane, PopupWindow, InputPopup, SelectionPopup from .parse import eds +from .meta import Meta # Key Constants not defined in curses # _UBUNTU key constants work in Ubuntu @@ -96,7 +97,7 @@ class App: """ def __init__(self: App, message_table: MessageTable, eds_configs: dict, - bus: MagicCANBus): + bus: MagicCANBus, meta: Meta): """ App Initialization function :param message_table: Reference to shared message table object @@ -107,6 +108,7 @@ def __init__(self: App, message_table: MessageTable, eds_configs: dict, self.bus = bus self.selected_pane_pos = 0 self.selected_pane = None + self.meta = meta self.key_dict = { KeyMap.UP_ARR.value['key']: self.up, KeyMap.S_UP_ARR.value['key']: self.shift_up, @@ -357,6 +359,7 @@ def handle_keyboard_input(self: App) -> None: value = self.add_if_win.get_value() if value != "": self.bus.add_interface(value) + self.meta.save_devices(self.bus) self.add_if_win.toggle() else: self.add_if_win.read_input(keyboard_input) @@ -367,6 +370,7 @@ def handle_keyboard_input(self: App) -> None: value = self.remove_if_win.get_value() if value != "": self.bus.remove_interface(value) + self.meta.save_devices(self.bus) self.remove_if_win.toggle() else: self.remove_if_win.read_input(keyboard_input) diff --git a/canopen_monitor/meta.py b/canopen_monitor/meta.py index 9c5dc2d..1819790 100644 --- a/canopen_monitor/meta.py +++ b/canopen_monitor/meta.py @@ -1,18 +1,32 @@ from .can import MagicCANBus +from os import path +import json class Meta: def __init__(self, config_dir, cache_dir): self.config_dir = config_dir self.cache_dir = cache_dir + self.interfaces_file = self.config_dir + '/interfaces.json' + self.nodes_file = self.config_dir + '/nodes.json' def save_devices(self, mcb: MagicCANBus) -> bool: - # TODO: Save the list of devices in MCB - pass + output = {'interfaces': mcb.interface_list} + with open(self.interfaces_file, "w") as f: + json.dump(output, f) + f.truncate() - def load_devices(self) -> [str]: - # TODO: Pass back list of interface names to populate MCB - pass + def load_devices(self, interface_args: [str]) -> [str]: + if not path.isfile(self.interfaces_file): + return interface_args + + with open(self.interfaces_file, "r") as f: + interface_config = json.load(f) + for interface in interface_config['interfaces']: + if interface not in interface_args: + interface_args.append(interface) + + return interface_args def load_node_overrides(self) -> dict: pass diff --git a/tests/spec_meta.py b/tests/spec_meta.py new file mode 100644 index 0000000..d3aa97f --- /dev/null +++ b/tests/spec_meta.py @@ -0,0 +1,128 @@ +import unittest +from unittest.mock import mock_open, patch, MagicMock, call +from canopen_monitor.meta import Meta + + +class TestMeta(unittest.TestCase): + """ + Tests for the Meta Class + """ + def setUp(self) -> None: + self.meta = Meta("config_dir", "cache_dir") + self.mcb = MagicMock() + + def test_save_single_device(self): + """ + Test Save a single Device + """ + self.mcb.interface_list = ["vcan0"] + with patch('builtins.open', mock_open()) as m: + self.meta.save_devices(self.mcb) + + m.assert_called_once_with('config_dir/interfaces.json', 'w') + calls = [call('{'), + call('"interfaces"'), + call(': '), + call('["vcan0"'), + call(']'), + call('}')] + + self.assertEqual(calls, m().write.mock_calls, + "json file not written out correctly") + + def test_save_multiple_device(self): + """ + Test Save Multiple Devices + """ + self.mcb.interface_list = ["vcan0", "vcan1"] + with patch('builtins.open', mock_open()) as m: + self.meta.save_devices(self.mcb) + + m.assert_called_once_with('config_dir/interfaces.json', 'w') + calls = [call('{'), + call('"interfaces"'), + call(': '), + call('["vcan0"'), + call(', "vcan1"'), + call(']'), + call('}')] + + self.assertEqual(calls, m().write.mock_calls, + "json file not written out correctly") + + def test_save_no_device(self): + """ + Test Save No Devices + """ + self.mcb.interface_list = [] + with patch('builtins.open', mock_open()) as m: + self.meta.save_devices(self.mcb) + + m.assert_called_once_with('config_dir/interfaces.json', 'w') + calls = [call('{'), + call('"interfaces"'), + call(': '), + call('[]'), + call('}')] + + self.assertEqual(calls, m().write.mock_calls, + "json file not written out correctly") + + m().truncate.assert_called_once() + + def test_load_single_device(self): + """ + Test load a single Device + """ + + json = '{"interfaces": ["vcan0"]}' + with patch('builtins.open', mock_open(read_data=json)) as m: + with patch('os.path.isfile', return_value=True) as m_os: + devices = self.meta.load_devices([]) + + m.assert_called_once_with('config_dir/interfaces.json', 'r') + m_os.assert_called_once_with('config_dir/interfaces.json') + self.assertEqual(["vcan0"], devices, + "Devices not loaded correctly") + + def test_load_multiple_devices(self): + """ + Test load of multiple Device + """ + + json = '{"interfaces": ["vcan0", "vcan1"]}' + with patch('builtins.open', mock_open(read_data=json)) as m: + with patch('os.path.isfile', return_value=True) as m_os: + devices = self.meta.load_devices([]) + + m.assert_called_once_with('config_dir/interfaces.json', 'r') + m_os.assert_called_once_with('config_dir/interfaces.json') + self.assertEqual(["vcan0", "vcan1"], devices, + "Devices not loaded correctly") + + def test_load_no_devices(self): + """ + Test load of no Devices + """ + + json = '{"interfaces": []}' + with patch('builtins.open', mock_open(read_data=json)) as m: + with patch('os.path.isfile', return_value=True) as m_os: + devices = self.meta.load_devices([]) + + m.assert_called_once_with('config_dir/interfaces.json', 'r') + m_os.assert_called_once_with('config_dir/interfaces.json') + self.assertEqual([], devices, + "Devices not loaded correctly") + + def test_load_no_file(self): + """ + Test load with no existing file + """ + + with patch('os.path.isfile', return_value=False) as m_os: + devices = self.meta.load_devices([]) + + m_os.assert_called_once_with('config_dir/interfaces.json') + self.assertEqual([], devices, + "Devices not loaded correctly") \ No newline at end of file From c20adcb27fbfe67fecaaaf1e8948b42dd0e26e2b Mon Sep 17 00:00:00 2001 From: Brian ONeill Date: Mon, 10 May 2021 13:20:04 -0700 Subject: [PATCH 6/7] Bugfix only load .eds and .dcf files --- canopen_monitor/__main__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/canopen_monitor/__main__.py b/canopen_monitor/__main__.py index 899daa8..5b6e169 100755 --- a/canopen_monitor/__main__.py +++ b/canopen_monitor/__main__.py @@ -17,8 +17,9 @@ def load_eds_files(filepath: str = CACHE_DIR) -> dict: configs = {} for file in os.listdir(filepath): full_path = f'{filepath}/{file}' - config = load_eds_file(full_path) - configs[config.node_id] = config + if file.lower().endswith(".eds") or file.lower().endswith(".dcf"): + config = load_eds_file(full_path) + configs[config.node_id] = config return configs From 4518e49d935263e92974afd23a28188e1e7c7765 Mon Sep 17 00:00:00 2001 From: dmitri-mcguckin Date: Mon, 10 May 2021 20:42:09 -0400 Subject: [PATCH 7/7] Update parser to quick-convert node ids to hex from dec --- canopen_monitor/parse/eds.py | 6 +++--- tests/__init__.py | 2 +- tests/spec_eds_parser.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/canopen_monitor/parse/eds.py b/canopen_monitor/parse/eds.py index 2a25858..bbdaf30 100644 --- a/canopen_monitor/parse/eds.py +++ b/canopen_monitor/parse/eds.py @@ -124,10 +124,10 @@ def __len__(self) -> int: def convert_value(value: str) -> Union[int, str]: # Turn number-like objects into numbers if (value != ''): - if (all(c in string.digits for c in value)): - return int(value, 10) - elif (all(c in string.hexdigits for c in value)): + if (all(c in string.hexdigits for c in value)): return int(value, 16) + elif (all(c in string.digits for c in value)): + return int(value, 10) else: return value diff --git a/tests/__init__.py b/tests/__init__.py index fc5c6ea..b93980e 100755 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -7750,4 +7750,4 @@ ParameterValue= PDOMapping=0 -""" \ No newline at end of file +""" diff --git a/tests/spec_eds_parser.py b/tests/spec_eds_parser.py index 19b155a..39da3fc 100644 --- a/tests/spec_eds_parser.py +++ b/tests/spec_eds_parser.py @@ -102,7 +102,7 @@ def test_get_node_id(self): """ DCF Parsing set node id attribute """ - self.assertEqual(10, + self.assertEqual(0x10, self.eds.node_id, "Error parsing node id")