From 7c3925e5f698efc6cca3ec22a672e2050288ba7d Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Mon, 21 Jun 2021 18:25:50 +0200 Subject: [PATCH 01/12] Correct output messages in `receive_store_dcmtk()` --- pynetdicom/tests/benchmark_script.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pynetdicom/tests/benchmark_script.py b/pynetdicom/tests/benchmark_script.py index 25f15a416b..96ddce053f 100755 --- a/pynetdicom/tests/benchmark_script.py +++ b/pynetdicom/tests/benchmark_script.py @@ -300,12 +300,12 @@ def receive_store_dcmtk(nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( - "C-STORE SCP transferred {} total datasets over {} " + "C-STORE DCMTK SCU/SCP transferred {} total datasets over {} " "association(s) in {:.2f} s" .format(nr_assoc * ds_per_assoc, nr_assoc, time.time() - start_time) ) else: - print("C-STORE SCP benchmark failed") + print("C-STORE DCMTK SCU/SCP benchmark failed") server.terminate() From 3f06631b5e3d67ed5d5130533a428cad00eb5598 Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Mon, 21 Jun 2021 18:29:18 +0200 Subject: [PATCH 02/12] Improve `benchmark_script.py` - Added `nr_ds` to make it easy to configure the number of datasets to use in the benchmark - Print the name of the DICOM file being used for the benchmark Note that `pynetdicom` performance is worse for larger DICOM files, such as `RTImageStorage.dcm`: ``` 9) C-STORE SCU/SCP transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 20.88 s 10) C-STORE DCMTK SCU/SCP transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 0.28 s ``` For `RTImageStorage.dcm`, `pynetdicom` is ~75x slower than DCMTK. --- pynetdicom/tests/benchmark_script.py | 68 ++++++++++++++++------------ 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/pynetdicom/tests/benchmark_script.py b/pynetdicom/tests/benchmark_script.py index 96ddce053f..1ddc8fed1a 100755 --- a/pynetdicom/tests/benchmark_script.py +++ b/pynetdicom/tests/benchmark_script.py @@ -183,9 +183,10 @@ def handle(event): if is_successful: print( - "C-STORE SCP transferred {} total datasets over {} " + "C-STORE SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, nr_assoc, time.time() - start_time) + .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + nr_assoc, time.time() - start_time) ) else: print("C-STORE SCP benchmark failed") @@ -258,9 +259,10 @@ def handle(event): if is_successful: print( - "C-STORE SCU/SCP transferred {} total datasets over {} " + "C-STORE SCU/SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, nr_assoc, time.time() - start_time) + .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + nr_assoc, time.time() - start_time) ) else: print("C-STORE SCU/SCP benchmark failed") @@ -300,9 +302,10 @@ def receive_store_dcmtk(nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( - "C-STORE DCMTK SCU/SCP transferred {} total datasets over {} " + "C-STORE DCMTK SCU/SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, nr_assoc, time.time() - start_time) + .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + nr_assoc, time.time() - start_time) ) else: print("C-STORE DCMTK SCU/SCP benchmark failed") @@ -360,9 +363,10 @@ def handle(event): if is_successful: print( - "C-STORE SCP transferred {} total datasets over {} " + "C-STORE SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, nr_assoc, time.time() - start_time) + .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + nr_assoc, time.time() - start_time) ) else: print("C-STORE SCP benchmark failed") @@ -427,9 +431,10 @@ def send_store(nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( - "C-STORE SCU transferred {} total datasets over {} " + "C-STORE SCU transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, nr_assoc, time.time() - start_time) + .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + nr_assoc, time.time() - start_time) ) else: print("C-STORE SCU benchmark failed") @@ -445,37 +450,40 @@ def send_store(nr_assoc, ds_per_assoc, use_yappi=False): else: use_yappi = False + nr_ds = 1000 + ds_name = os.path.basename(DATASET.filename) + print("Which benchmark do you wish to run?") - print(" 1. Storage SCU, 1000 datasets over 1 association") - print(" 2. Storage SCU, 1 dataset per association over 1000 associations") - print(" 3. Storage SCP, 1000 datasets over 1 association") - print(" 4. Storage SCP, 1000 datasets over 1 association (write)") - print(" 5. Storage SCP, 1000 datasets over 1 association (write fast)") - print(" 6. Storage SCP, 1000 datasets over 1 association (write fastest)") - print(" 7. Storage SCP, 1 dataset per association over 1000 associations") - print(" 8. Storage SCP, 1000 datasets per association over 10 simultaneous associations") - print(" 9. Storage SCU/SCP, 1000 datasets over 1 association") - print(" 10. Storage DCMTK SCU/SCP, 1000 datasets over 1 association") + print(" 1. Storage SCU, {} {} datasets over 1 association".format(nr_ds, ds_name)) + print(" 2. Storage SCU, 1 {} dataset per association over {} associations".format(nr_ds, ds_name)) + print(" 3. Storage SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) + print(" 4. Storage SCP, {} {} datasets over 1 association (write)".format(nr_ds, ds_name)) + print(" 5. Storage SCP, {} {} datasets over 1 association (write fast)".format(nr_ds, ds_name)) + print(" 6. Storage SCP, {} {} datasets over 1 association (write fastest)".format(nr_ds, ds_name)) + print(" 7. Storage SCP, 1 {} dataset per association over {} associations".format(nr_ds, ds_name)) + print(" 8. Storage SCP, {} {} datasets per association over 10 simultaneous associations".format(nr_ds, ds_name)) + print(" 9. Storage SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) + print(" 10. Storage DCMTK SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) bench_index = input() if bench_index == "1": - send_store(1, 1000, use_yappi) + send_store(1, nr_ds, use_yappi) elif bench_index == "2": - send_store(1000, 1, use_yappi) + send_store(nr_ds, 1, use_yappi) elif bench_index == "3": - receive_store(1, 1000, 0, use_yappi) + receive_store(1, nr_ds, 0, use_yappi) elif bench_index == "4": - receive_store(1, 1000, 1, use_yappi) + receive_store(1, nr_ds, 1, use_yappi) elif bench_index == "5": - receive_store(1, 1000, 2, use_yappi) + receive_store(1, nr_ds, 2, use_yappi) elif bench_index == "6": - receive_store(1, 1000, 3, use_yappi) + receive_store(1, nr_ds, 3, use_yappi) elif bench_index == "7": - receive_store(1000, 1, 0, use_yappi) + receive_store(nr_ds, 1, 0, use_yappi) elif bench_index == "8": - receive_store_simultaneous(10, 1000, use_yappi) + receive_store_simultaneous(10, nr_ds, use_yappi) elif bench_index == "9": - receive_store_internal(1, 1000, 0, use_yappi) + receive_store_internal(1, nr_ds, 0, use_yappi) elif bench_index == "10": - receive_store_dcmtk(1, 1000, use_yappi) + receive_store_dcmtk(1, nr_ds, use_yappi) From b7e1031378b84f6a92dd03450c2a8a9433b99af8 Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Mon, 21 Jun 2021 18:36:07 +0200 Subject: [PATCH 03/12] Don't sleep in `run_reactor()` if there are more events to process Instead of sleeping before every event, only sleep when there are no queued events to process. This ensures that queued events are processed as fast as possible. The `_run_loop_delay` setting now only affects the case where there are no queued events (i.e. the system is idle). After the performance optimization, `pynetdicom` is 23x faster than before when transferring `RTImageStorage.dcm`: New: ``` 1) C-STORE SCU transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 0.85 s 9) C-STORE SCU/SCP transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 1.43 s 10) C-STORE DCMTK SCU/SCP transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 0.29 s ``` Old: ``` 1) C-STORE SCU transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 19.69 s 9) C-STORE SCU/SCP transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 20.88 s 10) C-STORE DCMTK SCU/SCP transferred 100 total RTImageStorage.dcm datasets over 1 association(s) in 0.28 s ``` Now `pynetdicom` is only ~5x slower than DCMTK instead of ~75x. --- pynetdicom/dul.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pynetdicom/dul.py b/pynetdicom/dul.py index 902e3fa53f..b94924657b 100644 --- a/pynetdicom/dul.py +++ b/pynetdicom/dul.py @@ -378,14 +378,20 @@ def run_reactor(self) -> None: # Main DUL loop self._idle_timer.start() self.socket = cast("AssociationSocket", self.socket) + processed_event = True while True: # Let the assoc reactor off the leash if not self.assoc._dul_ready.is_set(): self.assoc._dul_ready.set() - # This effectively controls how quickly the DUL does anything - time.sleep(self._run_loop_delay) + if not processed_event: + # If there were no events to process on the previous loop, + # sleep before checking again. Otherwise check immediately. + + # Setting this higher will use less CPU when idle, but will also + # increases the latency to respond to new requests. + time.sleep(self._run_loop_delay) if self._kill_thread: break @@ -426,9 +432,11 @@ def run_reactor(self) -> None: event = self.event_queue.get(block=False) # If the queue is empty, return to the start of the loop except queue.Empty: + processed_event = False continue self.state_machine.do_action(event) + processed_event = True def send_pdu(self, primitive: _PDUPrimitiveType) -> None: """Place a primitive in the provider queue to be sent to the peer. From c52768d7b86fbd4416caaa77a1347769ee927fff Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Mon, 21 Jun 2021 18:36:35 +0200 Subject: [PATCH 04/12] Update .gitignore for PyCharm files --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index b50ed65c46..e03ca405d8 100644 --- a/.gitignore +++ b/.gitignore @@ -93,3 +93,6 @@ ENV/ # Rope project settings .ropeproject + +# PyCharm +.idea From 78504e7d33cc6fa4730dca6a37594f9dd9c21a14 Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Mon, 21 Jun 2021 23:28:32 +0200 Subject: [PATCH 05/12] Add __str__() methods to event classes to aid debugging --- pynetdicom/events.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pynetdicom/events.py b/pynetdicom/events.py index 1bc22266eb..d21a1f5e6d 100644 --- a/pynetdicom/events.py +++ b/pynetdicom/events.py @@ -84,6 +84,10 @@ class NotificationEvent(NamedTuple): is_intervention: bool = False is_notification: bool = True + def __str__(self) -> str: + """String representation of the class.""" + return self.name + # pylint: disable=line-too-long EVT_ABORTED = NotificationEvent("EVT_ABORTED", "Association aborted") @@ -135,6 +139,10 @@ class InterventionEvent(NamedTuple): is_intervention: bool = True is_notification: bool = False + def __str__(self) -> str: + """String representation of the class.""" + return self.name + EVT_ASYNC_OPS = InterventionEvent("EVT_ASYNC_OPS", "Asynchronous operations negotiation requested") # noqa EVT_SOP_COMMON = InterventionEvent("EVT_SOP_COMMON", "SOP class common extended negotiation requested") # noqa @@ -421,6 +429,11 @@ def __init__( ) setattr(self, kk, vv) + def __str__(self) -> str: + """String representation of the class.""" + return 'Event(event={}, message_id={}, timestamp={}, is_cancelled={})'\ + .format(self.event, self.message_id, self.timestamp, self.is_cancelled) + @property def action_information(self) -> Dataset: """Return an N-ACTION request's `Action Information` as a *pydicom* From 87255db0a99885d5f84e5ff17ab5c9552738791c Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Tue, 22 Jun 2021 07:58:08 +0200 Subject: [PATCH 06/12] Fix for failure in `test_fsm.py::TestState02::test_evt01()` When single-stepping the reactor, sleep between events so that test code has time to run. --- pynetdicom/dul.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pynetdicom/dul.py b/pynetdicom/dul.py index b94924657b..33f25605af 100644 --- a/pynetdicom/dul.py +++ b/pynetdicom/dul.py @@ -378,18 +378,21 @@ def run_reactor(self) -> None: # Main DUL loop self._idle_timer.start() self.socket = cast("AssociationSocket", self.socket) - processed_event = True + sleep = False while True: # Let the assoc reactor off the leash if not self.assoc._dul_ready.is_set(): self.assoc._dul_ready.set() + # When single-stepping the reactor, sleep between events so that + # test code has time to run. + sleep = True - if not processed_event: + if sleep: # If there were no events to process on the previous loop, # sleep before checking again. Otherwise check immediately. - # Setting this higher will use less CPU when idle, but will also + # Setting _run_loop_delay higher will use less CPU when idle, but will also # increases the latency to respond to new requests. time.sleep(self._run_loop_delay) @@ -432,11 +435,11 @@ def run_reactor(self) -> None: event = self.event_queue.get(block=False) # If the queue is empty, return to the start of the loop except queue.Empty: - processed_event = False + sleep = True continue self.state_machine.do_action(event) - processed_event = True + sleep = False def send_pdu(self, primitive: _PDUPrimitiveType) -> None: """Place a primitive in the provider queue to be sent to the peer. From 1059840ffd9281ffa148b25d40c9a05fa5a3e645 Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Tue, 22 Jun 2021 08:55:06 +0200 Subject: [PATCH 07/12] Improvements to `benchmark_script.py` - Prompt to use large or small dataset - Prompt for number of datasets to use (with default) - Run multiple benchmarks in a single run --- pynetdicom/tests/benchmark_script.py | 124 +++++++++++++++++---------- 1 file changed, 77 insertions(+), 47 deletions(-) diff --git a/pynetdicom/tests/benchmark_script.py b/pynetdicom/tests/benchmark_script.py index 1ddc8fed1a..01942ae2fc 100755 --- a/pynetdicom/tests/benchmark_script.py +++ b/pynetdicom/tests/benchmark_script.py @@ -16,6 +16,7 @@ from datetime import datetime import multiprocessing import os +import re import subprocess import tempfile import time @@ -32,8 +33,6 @@ TEST_DS_DIR = os.path.join(os.path.dirname(__file__), 'dicom_files') -#DATASET = dcmread(os.path.join(TEST_DS_DIR, 'RTImageStorage.dcm')) # 2.1 MB -DATASET = dcmread(os.path.join(TEST_DS_DIR, 'CTImageStorage.dcm')) # 39 kB def init_yappi(): @@ -119,11 +118,13 @@ def start_storescu(ds_per_assoc): return subprocess.Popen(args) -def receive_store(nr_assoc, ds_per_assoc, write_ds=0, use_yappi=False): +def receive_store(test_ds, nr_assoc, ds_per_assoc, write_ds=0, use_yappi=False): """Run a Storage SCP and transfer datasets with sequential storescu's. Parameters ---------- + test_ds : pydicom.dataset.Dataset + The test dataset to use nr_assoc : int The total number of (sequential) associations that will be made. ds_per_assoc : int @@ -160,7 +161,7 @@ def handle(event): ae.network_timeout = 5 if write_ds == 3: ae.maximum_pdu_size = 0 - ae.add_supported_context(DATASET.SOPClassUID, ImplicitVRLittleEndian) + ae.add_supported_context(test_ds.SOPClassUID, ImplicitVRLittleEndian) server = ae.start_server( ('', 11112), block=False, evt_handlers=[(evt.EVT_C_STORE, handle)] @@ -185,7 +186,7 @@ def handle(event): print( "C-STORE SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), nr_assoc, time.time() - start_time) ) else: @@ -194,11 +195,13 @@ def handle(event): server.shutdown() -def receive_store_internal(nr_assoc, ds_per_assoc, write_ds=0, use_yappi=False): +def receive_store_internal(test_ds, nr_assoc, ds_per_assoc, write_ds=0, use_yappi=False): """Run a Storage SCP and transfer datasets with pynetdicom alone. Parameters ---------- + test_ds : pydicom.dataset.Dataset + The test dataset to use nr_assoc : int The total number of (sequential) associations that will be made. ds_per_assoc : int @@ -235,8 +238,8 @@ def handle(event): ae.network_timeout = 5 if write_ds == 3: ae.maximum_pdu_size = 0 - ae.add_supported_context(DATASET.SOPClassUID, ImplicitVRLittleEndian) - ae.add_requested_context(DATASET.SOPClassUID, ImplicitVRLittleEndian) + ae.add_supported_context(test_ds.SOPClassUID, ImplicitVRLittleEndian) + ae.add_requested_context(test_ds.SOPClassUID, ImplicitVRLittleEndian) server = ae.start_server( ('', 11112), block=False, evt_handlers=[(evt.EVT_C_STORE, handle)] @@ -253,7 +256,7 @@ def handle(event): assoc = ae.associate('127.0.0.1', 11112) if assoc.is_established: for jj in range(ds_per_assoc): - assoc.send_c_store(DATASET) + assoc.send_c_store(test_ds) assoc.release() @@ -261,7 +264,7 @@ def handle(event): print( "C-STORE SCU/SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), nr_assoc, time.time() - start_time) ) else: @@ -270,11 +273,13 @@ def handle(event): server.shutdown() -def receive_store_dcmtk(nr_assoc, ds_per_assoc, use_yappi=False): +def receive_store_dcmtk(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): """Run a Storage SCP and transfer datasets with sequential storescu's. Parameters ---------- + test_ds : pydicom.dataset.Dataset + The test dataset to use nr_assoc : int The total number of (sequential) associations that will be made. ds_per_assoc : int @@ -304,21 +309,24 @@ def receive_store_dcmtk(nr_assoc, ds_per_assoc, use_yappi=False): print( "C-STORE DCMTK SCU/SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), nr_assoc, time.time() - start_time) ) else: print("C-STORE DCMTK SCU/SCP benchmark failed") server.terminate() + time.sleep(0.5) -def receive_store_simultaneous(nr_assoc, ds_per_assoc, use_yappi=False): +def receive_store_simultaneous(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): """Run a Storage SCP and transfer datasets with simultaneous storescu's. Parameters ---------- + test_ds : pydicom.dataset.Dataset + The test dataset to use nr_assoc : int The number of simultaneous associations that will be made. ds_per_assoc : int @@ -337,7 +345,7 @@ def handle(event): ae.dimse_timeout = 5 ae.network_timeout = 5 ae.maximum_associations = 15 - ae.add_supported_context(DATASET.SOPClassUID, ImplicitVRLittleEndian) + ae.add_supported_context(test_ds.SOPClassUID, ImplicitVRLittleEndian) server = ae.start_server( ('', 11112), block=False, evt_handlers=[(evt.EVT_C_STORE, handle)] @@ -365,7 +373,7 @@ def handle(event): print( "C-STORE SCP transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), nr_assoc, time.time() - start_time) ) else: @@ -374,11 +382,13 @@ def handle(event): server.shutdown() -def send_store(nr_assoc, ds_per_assoc, use_yappi=False): +def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): """Send a number of sequential C-STORE requests. Parameters ---------- + test_ds : pydicom.dataset.Dataset + The test dataset to use nr_assoc : int The total number of (sequential) associations that will be made. ds_per_assoc : int @@ -397,7 +407,7 @@ def send_store(nr_assoc, ds_per_assoc, use_yappi=False): ae.acse_timeout = 5 ae.dimse_timeout = 5 ae.network_timeout = 5 - ae.add_requested_context(DATASET.SOPClassUID, ImplicitVRLittleEndian) + ae.add_requested_context(test_ds.SOPClassUID, ImplicitVRLittleEndian) # Start timer start_time = time.time() @@ -414,7 +424,7 @@ def send_store(nr_assoc, ds_per_assoc, use_yappi=False): if assoc.is_established: for jj in range(ds_per_assoc): try: - status = assoc.send_c_store(DATASET) + status = assoc.send_c_store(test_ds) if status and status.Status != 0x0000: is_successful = False break @@ -433,57 +443,77 @@ def send_store(nr_assoc, ds_per_assoc, use_yappi=False): print( "C-STORE SCU transferred {} total {} datasets over {} " "association(s) in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(DATASET.filename), + .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), nr_assoc, time.time() - start_time) ) else: print("C-STORE SCU benchmark failed") server.terminate() + time.sleep(0.5) if __name__ == "__main__": - print("Use yappi (y/n:)") + print("Use yappi? (y/n:)") use_yappi = input() if use_yappi in ['y', 'Y']: use_yappi = True else: use_yappi = False - nr_ds = 1000 - ds_name = os.path.basename(DATASET.filename) + print("Use large dataset? (y/n:)") + use_large_dcm = input() + if use_large_dcm in ['y', 'Y']: + use_large_dcm = True + else: + use_large_dcm = False + + if use_large_dcm: + ds_name = 'RTImageStorage.dcm' # 2.1 MB + default_nr_ds = 100 + else: + ds_name = 'CTImageStorage.dcm' # 39 kB + default_nr_ds = 1000 + + print("number of datasets? (default = {})".format(default_nr_ds)) + try: + nr_ds = int(input()) + except ValueError: + nr_ds = default_nr_ds + + test_ds = dcmread(os.path.join(TEST_DS_DIR, ds_name)) - print("Which benchmark do you wish to run?") + print("Which benchmark(s) do you wish to run?") print(" 1. Storage SCU, {} {} datasets over 1 association".format(nr_ds, ds_name)) - print(" 2. Storage SCU, 1 {} dataset per association over {} associations".format(nr_ds, ds_name)) + print(" 2. Storage SCU, 1 {} dataset per association over {} associations".format(ds_name, nr_ds)) print(" 3. Storage SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) print(" 4. Storage SCP, {} {} datasets over 1 association (write)".format(nr_ds, ds_name)) print(" 5. Storage SCP, {} {} datasets over 1 association (write fast)".format(nr_ds, ds_name)) print(" 6. Storage SCP, {} {} datasets over 1 association (write fastest)".format(nr_ds, ds_name)) - print(" 7. Storage SCP, 1 {} dataset per association over {} associations".format(nr_ds, ds_name)) + print(" 7. Storage SCP, 1 {} dataset per association over {} associations".format(ds_name, nr_ds)) print(" 8. Storage SCP, {} {} datasets per association over 10 simultaneous associations".format(nr_ds, ds_name)) print(" 9. Storage SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) print(" 10. Storage DCMTK SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) - bench_index = input() - - if bench_index == "1": - send_store(1, nr_ds, use_yappi) - elif bench_index == "2": - send_store(nr_ds, 1, use_yappi) - elif bench_index == "3": - receive_store(1, nr_ds, 0, use_yappi) - elif bench_index == "4": - receive_store(1, nr_ds, 1, use_yappi) - elif bench_index == "5": - receive_store(1, nr_ds, 2, use_yappi) - elif bench_index == "6": - receive_store(1, nr_ds, 3, use_yappi) - elif bench_index == "7": - receive_store(nr_ds, 1, 0, use_yappi) - elif bench_index == "8": - receive_store_simultaneous(10, nr_ds, use_yappi) - elif bench_index == "9": - receive_store_internal(1, nr_ds, 0, use_yappi) - elif bench_index == "10": - receive_store_dcmtk(1, nr_ds, use_yappi) + bench_list = re.findall(r'\d+', input()) + + if "1" in bench_list: + send_store(test_ds, 1, nr_ds, use_yappi) + if "2" in bench_list: + send_store(test_ds, nr_ds, 1, use_yappi) + if "3" in bench_list: + receive_store(test_ds, 1, nr_ds, 0, use_yappi) + if "4" in bench_list: + receive_store(test_ds, 1, nr_ds, 1, use_yappi) + if "5" in bench_list: + receive_store(test_ds, 1, nr_ds, 2, use_yappi) + if "6" in bench_list: + receive_store(test_ds, 1, nr_ds, 3, use_yappi) + if "7" in bench_list: + receive_store(test_ds, nr_ds, 1, 0, use_yappi) + if "8" in bench_list: + receive_store_simultaneous(test_ds, 10, nr_ds, use_yappi) + if "9" in bench_list: + receive_store_internal(test_ds, 1, nr_ds, 0, use_yappi) + if "10" in bench_list: + receive_store_dcmtk(test_ds, 1, nr_ds, use_yappi) From f590b80aaa38ff927a6d266392b458ca5fbb11d9 Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Tue, 22 Jun 2021 10:34:54 +0200 Subject: [PATCH 08/12] More improvements to `benchmark_script.py` - Display write option in output of `receive_store()` - Support running all tests, ranges of tests, or lists of tests --- pynetdicom/tests/benchmark_script.py | 49 ++++++++++++++++++---------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/pynetdicom/tests/benchmark_script.py b/pynetdicom/tests/benchmark_script.py index 01942ae2fc..66635cd8e5 100755 --- a/pynetdicom/tests/benchmark_script.py +++ b/pynetdicom/tests/benchmark_script.py @@ -105,15 +105,17 @@ def start_storescp(): return subprocess.Popen(args) -def start_storescu(ds_per_assoc): +def start_storescu(test_ds, ds_per_assoc): """Run DCMTK's storescu in a background process. Parameters ---------- + test_ds : pydicom.dataset.Dataset + The test dataset to use ds_per_assoc : int The number of datasets to send using `storescu`. """ - fpath = os.path.join(TEST_DS_DIR, 'CTImageStorage.dcm') + fpath = test_ds.filename args = [which('storescu'), 'localhost', '11112'] + [fpath] * ds_per_assoc return subprocess.Popen(args) @@ -175,7 +177,7 @@ def handle(event): is_successful = True for ii in range(nr_assoc): - p = start_storescu(ds_per_assoc) + p = start_storescu(test_ds, ds_per_assoc) # Block until transfer is complete p.wait() if p.returncode != 0: @@ -183,11 +185,12 @@ def handle(event): break if is_successful: + write_msg = ["", " (write)", " (write fast)", " (write fastest)"][write_ds] print( "C-STORE SCP transferred {} total {} datasets over {} " - "association(s) in {:.2f} s" + "association{}{} in {:.2f} s" .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, time.time() - start_time) + nr_assoc, '' if nr_assoc == 1 else 's', write_msg, time.time() - start_time) ) else: print("C-STORE SCP benchmark failed") @@ -261,11 +264,12 @@ def handle(event): assoc.release() if is_successful: + write_msg = ["", " (write)", " (write fast)", " (write fastest)"][write_ds] print( "C-STORE SCU/SCP transferred {} total {} datasets over {} " - "association(s) in {:.2f} s" + "association{}{} in {:.2f} s" .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, time.time() - start_time) + nr_assoc, '' if nr_assoc == 1 else 's', write_msg, time.time() - start_time) ) else: print("C-STORE SCU/SCP benchmark failed") @@ -298,7 +302,7 @@ def receive_store_dcmtk(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): is_successful = True for ii in range(nr_assoc): - p = start_storescu(ds_per_assoc) + p = start_storescu(test_ds, ds_per_assoc) # Block until transfer is complete p.wait() if p.returncode != 0: @@ -308,9 +312,9 @@ def receive_store_dcmtk(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( "C-STORE DCMTK SCU/SCP transferred {} total {} datasets over {} " - "association(s) in {:.2f} s" + "association{} in {:.2f} s" .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, time.time() - start_time) + nr_assoc, '' if nr_assoc == 1 else 's', time.time() - start_time) ) else: print("C-STORE DCMTK SCU/SCP benchmark failed") @@ -360,7 +364,7 @@ def handle(event): processes = [] for ii in range(nr_assoc): - processes.append(start_storescu(ds_per_assoc)) + processes.append(start_storescu(test_ds, ds_per_assoc)) while None in [pp.poll() for pp in processes]: pass @@ -372,9 +376,9 @@ def handle(event): if is_successful: print( "C-STORE SCP transferred {} total {} datasets over {} " - "association(s) in {:.2f} s" + "association{} in {:.2f} s" .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, time.time() - start_time) + nr_assoc, '' if nr_assoc == 1 else 's', time.time() - start_time) ) else: print("C-STORE SCP benchmark failed") @@ -442,9 +446,9 @@ def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( "C-STORE SCU transferred {} total {} datasets over {} " - "association(s) in {:.2f} s" + "association{} in {:.2f} s" .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, time.time() - start_time) + nr_assoc, '' if nr_assoc == 1 else 's', time.time() - start_time) ) else: print("C-STORE SCU benchmark failed") @@ -483,7 +487,7 @@ def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): test_ds = dcmread(os.path.join(TEST_DS_DIR, ds_name)) - print("Which benchmark(s) do you wish to run?") + print("Which benchmarks do you wish to run? (list, range, or all)") print(" 1. Storage SCU, {} {} datasets over 1 association".format(nr_ds, ds_name)) print(" 2. Storage SCU, 1 {} dataset per association over {} associations".format(ds_name, nr_ds)) print(" 3. Storage SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) @@ -494,7 +498,18 @@ def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): print(" 8. Storage SCP, {} {} datasets per association over 10 simultaneous associations".format(nr_ds, ds_name)) print(" 9. Storage SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) print(" 10. Storage DCMTK SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) - bench_list = re.findall(r'\d+', input()) + + bench_input = input() + if re.fullmatch(r'\s*(a|all)\s*', bench_input): + # All: "a" or "all" + bench_list = [str(i) for i in range(1, 11)] + elif re.fullmatch(r'\s*(\d+)\s*-\s*(\d+)\s*', bench_input): + # Range: "x - y" + match = re.fullmatch(r'\s*(\d+)\s*-\s*(\d+)\s*', bench_input) + bench_list = [str(i) for i in range(int(match.group(1)), int(match.group(2)) + 1)] + else: + # List: "a, b, c" + bench_list = re.findall(r'\d+', bench_input) if "1" in bench_list: send_store(test_ds, 1, nr_ds, use_yappi) From e262b90e9f63191754d617db694254298492bfa6 Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Tue, 22 Jun 2021 17:51:23 +0200 Subject: [PATCH 09/12] Fix for failures in ``test_scp_cancelled()` unit tests - Copy working `sleep()` timing pattern from `TestUPSFindServiceClass::test_scp_cancelled()` to 3 instances of `test_scp_cancelled()` in `test_service_qr.py` --- pynetdicom/tests/test_service_qr.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pynetdicom/tests/test_service_qr.py b/pynetdicom/tests/test_service_qr.py index 57a927a303..1e56659c33 100644 --- a/pynetdicom/tests/test_service_qr.py +++ b/pynetdicom/tests/test_service_qr.py @@ -823,7 +823,7 @@ def handle(event): ds.PatientID = '123456' cancel_results.append(event.is_cancelled) yield 0xFF00, ds - time.sleep(0.2) + time.sleep(0.5) cancel_results.append(event.is_cancelled) yield 0xFE00, None yield 0xFF00, self.query @@ -843,6 +843,7 @@ def handle(event): results = assoc.send_c_find( identifier, PatientRootQueryRetrieveInformationModelFind, msg_id=11142 ) + time.sleep(0.2) assoc.send_c_cancel(1, 3) assoc.send_c_cancel(11142, 1) @@ -859,7 +860,7 @@ def handle(event): assoc.release() assert assoc.is_released - assert True in cancel_results + assert cancel_results == [False, True] scp.shutdown() @@ -2877,7 +2878,7 @@ def handle(event): yield 2 cancel_results.append(event.is_cancelled) yield 0xFF00, ds - time.sleep(0.2) + time.sleep(0.5) cancel_results.append(event.is_cancelled) yield 0xFE00, None @@ -2906,6 +2907,7 @@ def handle_store(event): results = assoc.send_c_get( identifier, PatientRootQueryRetrieveInformationModelGet, msg_id=11142 ) + time.sleep(0.2) assoc.send_c_cancel(1, 3) assoc.send_c_cancel(11142, 1) @@ -2926,7 +2928,7 @@ def handle_store(event): assoc.release() assert assoc.is_released - assert True in cancel_results + assert cancel_results == [False, True] scp.shutdown() @@ -4924,7 +4926,6 @@ def test_scp_cancelled(self): """Test is_cancelled works as expected.""" cancel_results = [] - attrs = {} def handle(event): ds = Dataset() ds.SOPClassUID = CTImageStorage @@ -4935,7 +4936,7 @@ def handle(event): yield 2 cancel_results.append(event.is_cancelled) yield 0xFF00, ds - time.sleep(0.2) + time.sleep(0.5) cancel_results.append(event.is_cancelled) yield 0xFE00, None @@ -4960,6 +4961,7 @@ def handle_store(event): PatientRootQueryRetrieveInformationModelMove, msg_id=11142 ) + time.sleep(0.2) assoc.send_c_cancel(1, 3) assoc.send_c_cancel(11142, 1) @@ -4980,7 +4982,7 @@ def handle_store(event): assoc.release() assert assoc.is_released - assert True in cancel_results + assert cancel_results == [False, True] scp.shutdown() From a5e92474f25cf36dfce90b62c76f184aeacd325b Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Thu, 24 Jun 2021 22:16:00 +0200 Subject: [PATCH 10/12] Address PR feedback - Add unit testing for `NotificationEvent::__str__()` - Add unit testing for `InterventionEvent::__str__()` - Remove `Event::__str__()` since it wasn't reliable/complete - Convert to f-strings in `benchmark_script.py` - Fix type error in `dsutils.py::pretty_dataset()` --- pynetdicom/dsutils.py | 3 +- pynetdicom/events.py | 5 --- pynetdicom/tests/benchmark_script.py | 64 ++++++++++++++-------------- pynetdicom/tests/test_events.py | 2 + 4 files changed, 35 insertions(+), 39 deletions(-) diff --git a/pynetdicom/dsutils.py b/pynetdicom/dsutils.py index 81aef3ba89..fa146bb07b 100644 --- a/pynetdicom/dsutils.py +++ b/pynetdicom/dsutils.py @@ -210,8 +210,7 @@ def pretty_dataset( list of str """ out = [] - for element in iter(ds): - elem = cast(DataElement, element) + for elem in iter(ds): if elem.VR == 'SQ': out.append(pretty_element(elem)) for ii, item in enumerate(elem.value): diff --git a/pynetdicom/events.py b/pynetdicom/events.py index d21a1f5e6d..62e091b4d6 100644 --- a/pynetdicom/events.py +++ b/pynetdicom/events.py @@ -429,11 +429,6 @@ def __init__( ) setattr(self, kk, vv) - def __str__(self) -> str: - """String representation of the class.""" - return 'Event(event={}, message_id={}, timestamp={}, is_cancelled={})'\ - .format(self.event, self.message_id, self.timestamp, self.is_cancelled) - @property def action_information(self) -> Dataset: """Return an N-ACTION request's `Action Information` as a *pydicom* diff --git a/pynetdicom/tests/benchmark_script.py b/pynetdicom/tests/benchmark_script.py index 66635cd8e5..4981db6505 100755 --- a/pynetdicom/tests/benchmark_script.py +++ b/pynetdicom/tests/benchmark_script.py @@ -187,10 +187,10 @@ def handle(event): if is_successful: write_msg = ["", " (write)", " (write fast)", " (write fastest)"][write_ds] print( - "C-STORE SCP transferred {} total {} datasets over {} " - "association{}{} in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, '' if nr_assoc == 1 else 's', write_msg, time.time() - start_time) + f"C-STORE SCP transferred {nr_assoc * ds_per_assoc} total " + f"{os.path.basename(test_ds.filename)} datasets over " + f"{nr_assoc} association{'' if nr_assoc == 1 else 's'}{write_msg} " + f"in {time.time() - start_time:.2f} s" ) else: print("C-STORE SCP benchmark failed") @@ -266,10 +266,10 @@ def handle(event): if is_successful: write_msg = ["", " (write)", " (write fast)", " (write fastest)"][write_ds] print( - "C-STORE SCU/SCP transferred {} total {} datasets over {} " - "association{}{} in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, '' if nr_assoc == 1 else 's', write_msg, time.time() - start_time) + f"C-STORE SCU/SCP transferred {nr_assoc * ds_per_assoc} total " + f"{os.path.basename(test_ds.filename)} datasets over " + f"{nr_assoc} association{'' if nr_assoc == 1 else 's'}{write_msg} " + f"in {time.time() - start_time:.2f} s" ) else: print("C-STORE SCU/SCP benchmark failed") @@ -311,10 +311,10 @@ def receive_store_dcmtk(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( - "C-STORE DCMTK SCU/SCP transferred {} total {} datasets over {} " - "association{} in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, '' if nr_assoc == 1 else 's', time.time() - start_time) + f"C-STORE DCMTK SCU/SCP transferred {nr_assoc * ds_per_assoc} total " + f"{os.path.basename(test_ds.filename)} datasets over " + f"{nr_assoc} association{'' if nr_assoc == 1 else 's'} " + f"in {time.time() - start_time:.2f} s" ) else: print("C-STORE DCMTK SCU/SCP benchmark failed") @@ -375,10 +375,10 @@ def handle(event): if is_successful: print( - "C-STORE SCP transferred {} total {} datasets over {} " - "association{} in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, '' if nr_assoc == 1 else 's', time.time() - start_time) + f"C-STORE SCP transferred {nr_assoc * ds_per_assoc} total " + f"{os.path.basename(test_ds.filename)} datasets over " + f"{nr_assoc} association{'' if nr_assoc == 1 else 's'} " + f"in {time.time() - start_time:.2f} s" ) else: print("C-STORE SCP benchmark failed") @@ -445,10 +445,10 @@ def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): if is_successful: print( - "C-STORE SCU transferred {} total {} datasets over {} " - "association{} in {:.2f} s" - .format(nr_assoc * ds_per_assoc, os.path.basename(test_ds.filename), - nr_assoc, '' if nr_assoc == 1 else 's', time.time() - start_time) + f"C-STORE SCU transferred {nr_assoc * ds_per_assoc} total " + f"{os.path.basename(test_ds.filename)} datasets over " + f"{nr_assoc} association{'' if nr_assoc == 1 else 's'} " + f"in {time.time() - start_time:.2f} s" ) else: print("C-STORE SCU benchmark failed") @@ -479,7 +479,7 @@ def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): ds_name = 'CTImageStorage.dcm' # 39 kB default_nr_ds = 1000 - print("number of datasets? (default = {})".format(default_nr_ds)) + print(f"number of datasets? (default = {default_nr_ds})") try: nr_ds = int(input()) except ValueError: @@ -487,17 +487,17 @@ def send_store(test_ds, nr_assoc, ds_per_assoc, use_yappi=False): test_ds = dcmread(os.path.join(TEST_DS_DIR, ds_name)) - print("Which benchmarks do you wish to run? (list, range, or all)") - print(" 1. Storage SCU, {} {} datasets over 1 association".format(nr_ds, ds_name)) - print(" 2. Storage SCU, 1 {} dataset per association over {} associations".format(ds_name, nr_ds)) - print(" 3. Storage SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) - print(" 4. Storage SCP, {} {} datasets over 1 association (write)".format(nr_ds, ds_name)) - print(" 5. Storage SCP, {} {} datasets over 1 association (write fast)".format(nr_ds, ds_name)) - print(" 6. Storage SCP, {} {} datasets over 1 association (write fastest)".format(nr_ds, ds_name)) - print(" 7. Storage SCP, 1 {} dataset per association over {} associations".format(ds_name, nr_ds)) - print(" 8. Storage SCP, {} {} datasets per association over 10 simultaneous associations".format(nr_ds, ds_name)) - print(" 9. Storage SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) - print(" 10. Storage DCMTK SCU/SCP, {} {} datasets over 1 association".format(nr_ds, ds_name)) + print(f"Which benchmarks do you wish to run? (list, range, or all)") + print(f" 1. Storage SCU, {nr_ds} {ds_name} datasets over 1 association") + print(f" 2. Storage SCU, 1 {ds_name} dataset per association over {nr_ds} associations") + print(f" 3. Storage SCP, {nr_ds} {ds_name} datasets over 1 association") + print(f" 4. Storage SCP, {nr_ds} {ds_name} datasets over 1 association (write)") + print(f" 5. Storage SCP, {nr_ds} {ds_name} datasets over 1 association (write fast)") + print(f" 6. Storage SCP, {nr_ds} {ds_name} datasets over 1 association (write fastest)") + print(f" 7. Storage SCP, 1 {ds_name} dataset per association over {nr_ds} associations") + print(f" 8. Storage SCP, {nr_ds} {ds_name} datasets per association over 10 simultaneous associations") + print(f" 9. Storage SCU/SCP, {nr_ds} {ds_name} datasets over 1 association") + print(f" 10. Storage DCMTK SCU/SCP, {nr_ds} {ds_name} datasets over 1 association") bench_input = input() if re.fullmatch(r'\s*(a|all)\s*', bench_input): diff --git a/pynetdicom/tests/test_events.py b/pynetdicom/tests/test_events.py index 26b06763f3..d4d687b12a 100644 --- a/pynetdicom/tests/test_events.py +++ b/pynetdicom/tests/test_events.py @@ -42,6 +42,7 @@ def test_intervention_namedtuple(): assert event.description == 'some description' assert event.is_intervention is True assert event.is_notification is False + assert str(event) == event.name def test_notification_namedtuple(): """Test the NotificationEvent namedtuple.""" @@ -50,6 +51,7 @@ def test_notification_namedtuple(): assert event.description == 'some description' assert event.is_intervention is False assert event.is_notification is True + assert str(event) == event.name def test_intervention_global(): """Test the _INTERVENTION_EVENTS global.""" From 159b8f4ac61135ee0dc51f6c005a65bb11288c7c Mon Sep 17 00:00:00 2001 From: Matthew Self Date: Tue, 29 Jun 2021 11:16:41 +0200 Subject: [PATCH 11/12] Add test coverage for `stop_dul()` Test that `stop_dul()` returns `True` when in `Sta1` --- pynetdicom/tests/test_dul.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pynetdicom/tests/test_dul.py b/pynetdicom/tests/test_dul.py index e8be965179..ad08cc0430 100644 --- a/pynetdicom/tests/test_dul.py +++ b/pynetdicom/tests/test_dul.py @@ -269,3 +269,9 @@ def patch_read_pdu(): scp.step() scp.shutdown() + + def test_stop_dul_sta1(self): + """Test that stop_dul() returns True when in Sta1""" + dul = DULServiceProvider(DummyAssociation()) + assert(dul.state_machine.current_state == 'Sta1') + assert(dul.stop_dul()) From c761804ab1f2c951dcb2628da8871db04bcd02e9 Mon Sep 17 00:00:00 2001 From: scaramallion Date: Wed, 22 Dec 2021 11:27:25 +1100 Subject: [PATCH 12/12] Add test for stop_dul --- pynetdicom/dul.py | 9 ++++----- pynetdicom/tests/test_dul.py | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/pynetdicom/dul.py b/pynetdicom/dul.py index e067c5788d..45848739cf 100644 --- a/pynetdicom/dul.py +++ b/pynetdicom/dul.py @@ -398,10 +398,9 @@ def run_reactor(self) -> None: if sleep: # If there were no events to process on the previous loop, - # sleep before checking again. Otherwise check immediately. - - # Setting _run_loop_delay higher will use less CPU when idle, but will also - # increases the latency to respond to new requests. + # sleep before checking again, otherwise check immediately + # Setting `_run_loop_delay` higher will use less CPU when idle, but + # will also increase the latency to respond to new requests time.sleep(self._run_loop_delay) if self._kill_thread: @@ -489,7 +488,7 @@ def stop_dul(self) -> bool: # Fix for Issue 39 # Give the DUL thread time to exit while self.is_alive(): - time.sleep(0.001) + time.sleep(self._run_loop_delay) return True diff --git a/pynetdicom/tests/test_dul.py b/pynetdicom/tests/test_dul.py index 2e97508a59..dabb908207 100644 --- a/pynetdicom/tests/test_dul.py +++ b/pynetdicom/tests/test_dul.py @@ -19,6 +19,7 @@ A_ABORT_RQ, ) from pynetdicom.pdu_primitives import A_ASSOCIATE, A_RELEASE, A_ABORT, P_DATA +from pynetdicom.sop_class import Verification from .encoded_pdu_items import a_associate_ac, a_release_rq from .parrot import start_server, ThreadedParrot, ParrotRequest from .utils import sleep @@ -70,6 +71,7 @@ class TestDUL: def setup(self): self.scp = None + self.ae = None def teardown(self): if self.scp: @@ -78,6 +80,9 @@ def teardown(self): self.scp.commands = [] self.scp.shutdown() + if self.ae: + self.ae.shutdown() + for thread in threading.enumerate(): if isinstance(thread, ThreadedParrot): thread.shutdown() @@ -296,3 +301,24 @@ def test_stop_dul_sta1(self): dul = DULServiceProvider(DummyAssociation()) assert dul.state_machine.current_state == "Sta1" assert dul.stop_dul() + + def test_stop_dul(self): + self.ae = ae = AE() + ae.network_timeout = 5 + ae.dimse_timeout = 5 + ae.acse_timeout = 5 + ae.add_supported_context(Verification) + + scp = ae.start_server(("", 11112), block=False) + + ae.add_requested_context(Verification) + assoc = ae.associate("localhost", 11112) + + dul = assoc.dul + + dul.state_machine.current_state = "Sta1" + dul.stop_dul() + + assoc.release() + + scp.shutdown()