diff --git a/canopen/objectdictionary/__init__.py b/canopen/objectdictionary/__init__.py index f900f71c..17081470 100644 --- a/canopen/objectdictionary/__init__.py +++ b/canopen/objectdictionary/__init__.py @@ -48,6 +48,10 @@ def export_od(od, dest:Union[str,TextIO,None]=None, doc_type:Optional[str]=None) from . import eds return eds.export_dcf(od, dest) + # If dest is opened in this fn, it should be closed + if type(dest) is str: + dest.close() + def import_od( source: Union[str, TextIO, None], diff --git a/canopen/objectdictionary/eds.py b/canopen/objectdictionary/eds.py index c1c54d78..0beb5158 100644 --- a/canopen/objectdictionary/eds.py +++ b/canopen/objectdictionary/eds.py @@ -33,7 +33,11 @@ def import_eds(source, node_id): except AttributeError: # Python 2 eds.readfp(fp) - fp.close() + finally: + # Only close object if opened in this fn + if not hasattr(source, "read"): + fp.close() + od = objectdictionary.ObjectDictionary() if eds.has_section("FileInfo"): @@ -181,8 +185,8 @@ def import_from_node(node_id, network): network.subscribe(0x580 + node_id, sdo_client.on_response) # Create file like object for Store EDS variable try: - eds_fp = sdo_client.open(0x1021, 0, "rt") - od = import_eds(eds_fp, node_id) + with sdo_client.open(0x1021, 0, "rt") as eds_fp: + od = import_eds(eds_fp, node_id) except Exception as e: logger.error("No object dictionary could be loaded for node %d: %s", node_id, e) diff --git a/canopen/sdo/client.py b/canopen/sdo/client.py index 7e0f58bf..7faa103a 100644 --- a/canopen/sdo/client.py +++ b/canopen/sdo/client.py @@ -114,9 +114,9 @@ def upload(self, index: int, subindex: int) -> bytes: :raises canopen.SdoAbortedError: When node responds with an error. """ - fp = self.open(index, subindex, buffering=0) - size = fp.size - data = fp.read() + with self.open(index, subindex, buffering=0) as fp: + size = fp.size + data = fp.read() if size is None: # Node did not specify how many bytes to use # Try to find out using Object Dictionary @@ -155,10 +155,9 @@ def download( :raises canopen.SdoAbortedError: When node responds with an error. """ - fp = self.open(index, subindex, "wb", buffering=7, size=len(data), - force_segment=force_segment) - fp.write(data) - fp.close() + with self.open(index, subindex, "wb", buffering=7, size=len(data), + force_segment=force_segment) as fp: + fp.write(data) def open(self, index, subindex=0, mode="rb", encoding="ascii", buffering=1024, size=None, block_transfer=False, force_segment=False, request_crc_support=True): diff --git a/doc/sdo.rst b/doc/sdo.rst index bfc78c25..c0db4ca5 100644 --- a/doc/sdo.rst +++ b/doc/sdo.rst @@ -72,14 +72,11 @@ Variables can be opened as readable or writable file objects which can be useful when dealing with large amounts of data:: # Open the Store EDS variable as a file like object - infile = node.sdo[0x1021].open('r', encoding='ascii') - # Open a file for writing to - outfile = open('out.eds', 'w', encoding='ascii') - # Iteratively read lines from node and write to file - outfile.writelines(infile) - # Clean-up - infile.close() - outfile.close() + with node.sdo[0x1021].open('r', encoding='ascii') as infile, + open('out.eds', 'w', encoding='ascii') as outfile: + + # Iteratively read lines from node and write to file + outfile.writelines(infile) Most APIs accepting file objects should also be able to accept this. @@ -88,17 +85,16 @@ server supports it. This is done through the file object interface:: FIRMWARE_PATH = '/path/to/firmware.bin' FILESIZE = os.path.getsize(FIRMWARE_PATH) - infile = open(FIRMWARE_PATH, 'rb') - outfile = node.sdo['Firmware'].open('wb', size=FILESIZE, block_transfer=True) - - # Iteratively transfer data without having to read all into memory - while True: - data = infile.read(1024) - if not data: - break - outfile.write(data) - infile.close() - outfile.close() + + with open(FIRMWARE_PATH, 'rb') as infile, + node.sdo['Firmware'].open('wb', size=FILESIZE, block_transfer=True) as outfile: + + # Iteratively transfer data without having to read all into memory + while True: + data = infile.read(1024) + if not data: + break + outfile.write(data) .. warning:: Block transfer is still in experimental stage! diff --git a/test/test_eds.py b/test/test_eds.py index 2a6d5098..79348616 100644 --- a/test/test_eds.py +++ b/test/test_eds.py @@ -15,7 +15,8 @@ def test_load_nonexisting_file(self): canopen.import_od('/path/to/wrong_file.eds') def test_load_file_object(self): - od = canopen.import_od(open(EDS_PATH)) + with open(EDS_PATH) as fp: + od = canopen.import_od(fp) self.assertTrue(len(od) > 0) def test_variable(self): diff --git a/test/test_local.py b/test/test_local.py index f4119d44..493cef1b 100644 --- a/test/test_local.py +++ b/test/test_local.py @@ -40,7 +40,8 @@ def test_expedited_upload(self): def test_block_upload_switch_to_expedite_upload(self): with self.assertRaises(canopen.SdoCommunicationError) as context: - self.remote_node.sdo[0x1008].open('r', block_transfer=True) + with self.remote_node.sdo[0x1008].open('r', block_transfer=True) as fp: + pass # We get this since the sdo client don't support the switch # from block upload to expedite upload self.assertEqual("Unexpected response 0x41", str(context.exception)) @@ -48,9 +49,10 @@ def test_block_upload_switch_to_expedite_upload(self): def test_block_download_not_supported(self): data = b"TEST DEVICE" with self.assertRaises(canopen.SdoAbortedError) as context: - self.remote_node.sdo[0x1008].open('wb', - size=len(data), - block_transfer=True) + with self.remote_node.sdo[0x1008].open('wb', + size=len(data), + block_transfer=True) as fp: + pass self.assertEqual(context.exception.code, 0x05040001) def test_expedited_upload_default_value_visible_string(self): diff --git a/test/test_sdo.py b/test/test_sdo.py index 67115499..c0ba086b 100644 --- a/test/test_sdo.py +++ b/test/test_sdo.py @@ -110,10 +110,9 @@ def test_block_download(self): (RX, b'\xa1\x00\x00\x00\x00\x00\x00\x00') ] data = b'A really really long string...' - fp = self.network[2].sdo['Writable string'].open( - 'wb', size=len(data), block_transfer=True) - fp.write(data) - fp.close() + with self.network[2].sdo['Writable string'].open( + 'wb', size=len(data), block_transfer=True) as fp: + fp.write(data) def test_block_upload(self): self.data = [ @@ -128,9 +127,8 @@ def test_block_upload(self): (RX, b'\xc9\x40\xe1\x00\x00\x00\x00\x00'), (TX, b'\xa1\x00\x00\x00\x00\x00\x00\x00') ] - fp = self.network[2].sdo[0x1008].open('r', block_transfer=True) - data = fp.read() - fp.close() + with self.network[2].sdo[0x1008].open('r', block_transfer=True) as fp: + data = fp.read() self.assertEqual(data, 'Tiny Node - Mega Domains !') def test_writable_file(self): @@ -144,10 +142,9 @@ def test_writable_file(self): (TX, b'\x0f\x00\x00\x00\x00\x00\x00\x00'), (RX, b'\x20\x00\x20\x00\x00\x00\x00\x00') ] - fp = self.network[2].sdo['Writable string'].open('wb') - fp.write(b'1234') - fp.write(b'56789') - fp.close() + with self.network[2].sdo['Writable string'].open('wb') as fp: + fp.write(b'1234') + fp.write(b'56789') self.assertTrue(fp.closed) # Write on closed file with self.assertRaises(ValueError):