some fixes and improvements

python_app/hardware_full/librevna_multi_device_driver/controller.py

@@ -216,11 +216,13 @@ class MultiDeviceVnaController:
 
     def _configure_reference_clocks(self) -> None:
         for device_connection in self._all_devices:
+            # 1 s ACK timeout plus one retry caps worst-case at ~2 s per device
+            # so a stuck reference apply cannot stall recovery for minutes.
             self._send_command_and_wait_for_acknowledgement(
                 device_connection,
                 PacketType.REFERENCE_SETTINGS,
                 build_reference_settings_payload(0, self._force_external_reference),
-                timeout_seconds=3.0,
+                timeout_seconds=1.0,
                 retry_count=1,
             )
 
@@ -241,6 +243,8 @@ class MultiDeviceVnaController:
             (self._master_device, True),
         ]
         for device_connection, is_synchronization_master in sweep_configuration_commands:
+            # 1 s ACK timeout plus one retry caps worst-case at ~2 s per device
+            # so a stuck sweep apply cannot stall recovery for minutes.
             self._send_command_and_wait_for_acknowledgement(
                 device_connection,
                 PacketType.SWEEP_SETTINGS,
@@ -250,7 +254,7 @@ class MultiDeviceVnaController:
                     synchronization_enabled=self._synchronization_enabled,
                     master_stimulus_ports=master_stimulus_ports,
                 ),
-                timeout_seconds=3.0,
+                timeout_seconds=1.0,
                 retry_count=1,
             )
         self._last_applied_sweep_configuration = replace(sweep_configuration)

python_app/hardware_full/librevna_multi_device_driver/cycle_collection.py

@@ -23,6 +23,19 @@ from python_app.hardware_full.librevna_multi_device_driver.transport import Libr
 
 LIBREVNA_NATIVE_SWEEP_TIMEOUT_SECONDS = 1.5
 
+# Hard upper bound on how long one full sweep cycle is allowed to take from
+# the moment the collection thread enters its loop. Even a device that keeps
+# streaming valid-looking datapoints will be abandoned once this deadline
+# elapses, so the caller's recovery loop can re-open it instead of waiting
+# forever. Kept comfortably above the worst real-world cycle (≈ points / IFBW).
+_MAX_FULL_CYCLE_SECONDS = 8.0
+
+# Maximum time we let collection threads linger after `stop_collection_requested`
+# has been set. They are all daemon threads and self-poll the flag every
+# ~0.2 s, so a 2 s grace period is generous. Past this point we stop joining
+# and let the orphan thread die when the producer process exits.
+_THREAD_JOIN_TIMEOUT_SECONDS = 2.0
+
 
 def collect_complete_running_sweep_cycles(
     *,
@@ -82,8 +95,13 @@ def collect_complete_running_sweep_cycles(
     ) -> None:
         datapoints_received = 0
         expected_datapoint_count = cycle_count * point_count
-        last_datapoint_timestamp = time.monotonic()
-        collection_loop_start = last_datapoint_timestamp
+        loop_start_timestamp = time.monotonic()
+        # Tracks the last time we actually accepted a datapoint into the cycle.
+        # Crucially, *not* updated on rejected datapoints — a device that keeps
+        # streaming valid-looking frames the handler ignores (e.g. waiting on
+        # point_index=0, or after cycle_count has been reached) must still hit
+        # the per-device timeout and trigger recovery instead of looping forever.
+        last_consumed_timestamp = loop_start_timestamp
         has_consumed_any_datapoint = False
 
         while datapoints_received < expected_datapoint_count:
@@ -91,19 +109,19 @@ def collect_complete_running_sweep_cycles(
                 return
 
             now = time.monotonic()
-            remaining_timeout_seconds = (last_datapoint_timestamp + datapoint_timeout_seconds) - now
+            remaining_timeout_seconds = (last_consumed_timestamp + datapoint_timeout_seconds) - now
             if remaining_timeout_seconds <= 0:
                 collection_errors.append(
                     TimeoutError(
-                        f"No datapoints from {device_connection.serial_number} for "
+                        f"No usable datapoints from {device_connection.serial_number} for "
                         f"{datapoint_timeout_seconds:.1f} s "
-                        f"(received {datapoints_received}/{point_count})"
+                        f"(received {datapoints_received}/{expected_datapoint_count})"
                     )
                 )
                 stop_collection_requested.set()
                 return
 
-            if not has_consumed_any_datapoint and (now - collection_loop_start) > cycle_start_guard_seconds:
+            if not has_consumed_any_datapoint and (now - loop_start_timestamp) > cycle_start_guard_seconds:
                 collection_errors.append(
                     TimeoutError(
                         f"Device {device_connection.serial_number} streamed datapoints but never "
@@ -114,6 +132,22 @@ def collect_complete_running_sweep_cycles(
                 stop_collection_requested.set()
                 return
 
+            # Hard wallclock deadline for the entire cycle. Even if every
+            # datapoint refreshes `last_consumed_timestamp` and the per-packet
+            # timeout never trips, we still bail out once the cycle has dragged
+            # on for too long — this is the safety net the per-device timeout
+            # cannot provide by itself.
+            if (now - loop_start_timestamp) > _MAX_FULL_CYCLE_SECONDS:
+                collection_errors.append(
+                    TimeoutError(
+                        f"Device {device_connection.serial_number} did not finish a sweep cycle "
+                        f"within {_MAX_FULL_CYCLE_SECONDS:.1f} s "
+                        f"(received {datapoints_received}/{expected_datapoint_count})"
+                    )
+                )
+                stop_collection_requested.set()
+                return
+
             try:
                 packet_type, payload = device_connection.receive_packet(
                     timeout_seconds=min(1.0, remaining_timeout_seconds)
@@ -136,9 +170,11 @@ def collect_complete_running_sweep_cycles(
 
             parsed_datapoint = parse_vna_datapoint_payload(payload)
             if parsed_datapoint and 0 <= parsed_datapoint.point_index < point_count:
-                last_datapoint_timestamp = time.monotonic()
                 datapoint_was_consumed = handle_datapoint(parsed_datapoint)
                 if datapoint_was_consumed:
+                    # Only refreshed on accepted datapoints so the no-progress
+                    # timeout above stays honest about real cycle progress.
+                    last_consumed_timestamp = time.monotonic()
                     has_consumed_any_datapoint = True
                     datapoint_counts_by_device_serial[device_connection.serial_number] += 1
                     datapoints_received += 1
@@ -269,8 +305,35 @@ def collect_complete_running_sweep_cycles(
 
     for collection_thread in collection_threads:
         collection_thread.start()
+
+    # Bounded join. Threads self-poll `stop_collection_requested` at most every
+    # ~0.2 s (the queue.get timeout inside `receive_packet`), so a 2 s grace
+    # period is more than enough for a cooperative shutdown. Anything still
+    # alive after that is treated as an orphan: we set the flag a second time,
+    # record an error so callers go through recovery, and stop waiting. The
+    # thread is a daemon and will die with the producer process.
+    deadline = time.monotonic() + _THREAD_JOIN_TIMEOUT_SECONDS
     for collection_thread in collection_threads:
-        collection_thread.join()
+        remaining_seconds = deadline - time.monotonic()
+        collection_thread.join(timeout=max(0.0, remaining_seconds))
+
+    stalled_threads = [
+        collection_thread for collection_thread in collection_threads if collection_thread.is_alive()
+    ]
+    if stalled_threads:
+        stop_collection_requested.set()
+        # Give them one more short window in case they were just slow to react.
+        secondary_deadline = time.monotonic() + 0.5
+        for stalled_thread in stalled_threads:
+            stalled_thread.join(timeout=max(0.0, secondary_deadline - time.monotonic()))
+        still_stalled = [stalled_thread for stalled_thread in stalled_threads if stalled_thread.is_alive()]
+        if still_stalled:
+            collection_errors.append(
+                RuntimeError(
+                    "Sweep collector thread(s) failed to stop within the join deadline: "
+                    + ", ".join(stalled_thread.name for stalled_thread in still_stalled)
+                )
+            )
 
     if collection_errors:
         raise RuntimeError(f"Sweep collection failed: {collection_errors[0]}") from collection_errors[0]