From c282486b222cbfa027eaeb9e049db9a39d1e67cd Mon Sep 17 00:00:00 2001 From: Carl Chalmers Date: Tue, 28 Jul 2026 12:16:49 +0100 Subject: [PATCH] Address CodeQL code-quality alerts - Narrow 10 broad `except Exception: pass` blocks to the specific exception types actually raised (OSError / serial.SerialException) and add an intent comment to each. Two other empty-except blocks were already narrow; those just get a comment. - Remove dead code: `IMAGE_EXTS`, `K`, `sys_path`, `proc_path`, and the `global clf_ort_sessions` no-op declaration. - Trim 3 dead `nonlocal` declarations in xbee_master_collect.py where the names were only read or mutated (never rebound). - Replace 2 top-level `exit(1)` calls with `sys.exit(1)` in rest_client.py. - Drop the redundant `from torch import Tensor` in detection_utils.py and use `torch.Tensor` in the 4 annotation sites instead. No functional changes. Verified end-to-end on the test Pi: sparrow container recreated with the new code, no ImportError/traceback in any log, and an injected test JPEG round-trips through inference and uploads to the server with 200. --- sparrow/audio.py | 3 ++- sparrow/email_server.py | 1 + sparrow/inference.py | 1 - sparrow/model_update.py | 4 +++- sparrow/rest_client.py | 13 +++++++------ sparrow/utils/detection_utils.py | 10 ++++------ sparrow/xbee_configure.py | 3 ++- sparrow/xbee_master_collect.py | 28 +++++++++++++++++----------- 8 files changed, 36 insertions(+), 27 deletions(-) diff --git a/sparrow/audio.py b/sparrow/audio.py index a0815f6..06a0641 100644 --- a/sparrow/audio.py +++ b/sparrow/audio.py @@ -765,7 +765,8 @@ def monitor_audio(): finally: try: os.remove(temp_file) - except Exception: + except OSError: + # Temp file may already be gone or unremovable; nothing to do. pass diff --git a/sparrow/email_server.py b/sparrow/email_server.py index 95a8cf8..08dc715 100644 --- a/sparrow/email_server.py +++ b/sparrow/email_server.py @@ -107,6 +107,7 @@ def factory(self): try: asyncio.get_event_loop().run_forever() except KeyboardInterrupt: + # Graceful shutdown on Ctrl+C; the finally block stops the controller. pass finally: controller.stop() diff --git a/sparrow/inference.py b/sparrow/inference.py index 710856c..75d9dd3 100644 --- a/sparrow/inference.py +++ b/sparrow/inference.py @@ -303,7 +303,6 @@ def _get_megadetector_onnx_session(): def _get_classifier_onnx_session(model_name: str): - global clf_ort_sessions if model_name in clf_ort_sessions: return clf_ort_sessions[model_name] if not _ensure_ort_ready(): diff --git a/sparrow/model_update.py b/sparrow/model_update.py index b45d1cc..ef09d52 100644 --- a/sparrow/model_update.py +++ b/sparrow/model_update.py @@ -70,7 +70,8 @@ def robust_download_file(file_url: str, local_file_path: str): if os.path.exists(temp_file_path): try: os.remove(temp_file_path) - except Exception: + except OSError: + # Best-effort cleanup of partial download; original exception is re-raised below. pass raise @@ -178,6 +179,7 @@ def sync_models(model_data: dict): os.remove(fpath) log.info(f"Removed extra file: {fpath}") except FileNotFoundError: + # File already gone (races with a concurrent cleanup); OK. pass except Exception as e: log.warning(f"Failed to remove {fpath}: {e}") diff --git a/sparrow/rest_client.py b/sparrow/rest_client.py index 2afcb5e..40a2e49 100644 --- a/sparrow/rest_client.py +++ b/sparrow/rest_client.py @@ -7,6 +7,7 @@ """ import os +import sys import time import requests import csv @@ -55,20 +56,17 @@ auth_key = f.read().strip() except Exception as e: logger.error(f"Failed to read access key from /app/config/access_key.txt: {e}") - exit(1) + sys.exit(1) metrics_backlog_file = "/app/static/data/metrics_backlog.jsonl" -sys_path = os.getenv("SYS_PATH", "/sys") -proc_path = os.getenv("PROC_PATH", "/proc") - # Generate Unique ID try: unique_id = get_hardware_id() logger.info(f"Generated unique_id: {unique_id}") except Exception: logger.critical("Cannot proceed without a valid unique_id.") - exit(1) + sys.exit(1) # VE.Direct (Solar). Use a Victron-specific by-id symlink so this never grabs # the XBee FTDI adapter. When the VE.Direct cable isn't plugged in, the symlink @@ -365,7 +363,9 @@ def remove_records_from_csv(image_name): logger.error(f"CSV update failed: {e}") try: os.remove(tmp) - except Exception: + except OSError: + # Best-effort cleanup of the temp file — it may already be gone + # (e.g. the initial write failed) or unremovable; nothing to do here. pass def process_and_upload_images(): @@ -416,6 +416,7 @@ def process_and_upload_images(): os.remove(side_path) logger.info(f"Deleted sidecar: {side_path}") except FileNotFoundError: + # Sidecar is optional; nothing to remove if it was never written. pass except Exception as e: logger.error(f"Failed to delete sidecar {side_path}: {e}") diff --git a/sparrow/utils/detection_utils.py b/sparrow/utils/detection_utils.py index 3043c19..3172f0f 100644 --- a/sparrow/utils/detection_utils.py +++ b/sparrow/utils/detection_utils.py @@ -5,11 +5,10 @@ from typing import List, Optional, Sequence, Tuple, Union import numpy as np import torch -from torch import Tensor from torchvision.ops import nms as tv_nms from torchvision.ops import batched_nms as tv_batched_nms -ArrayLike = Union[np.ndarray, Tensor] +ArrayLike = Union[np.ndarray, torch.Tensor] def _is_tensor(x: ArrayLike) -> bool: @@ -102,14 +101,14 @@ def scale_boxes( @torch.no_grad() def non_max_suppression( - prediction: Tensor, + prediction: torch.Tensor, conf_thres: float = 0.25, iou_thres: float = 0.45, classes: Optional[Sequence[int]] = None, agnostic: bool = False, max_det: int = 300, max_time_img: float = 0.05, -) -> List[Tensor]: +) -> List[torch.Tensor]: """ Generic NMS that supports multiple common formats and **auto-detects layout**: @@ -141,7 +140,7 @@ def non_max_suppression( B, N, C = prediction.shape t_start = time.time() - outputs: List[Tensor] = [] + outputs: List[torch.Tensor] = [] for b in range(B): x = prediction[b] @@ -172,7 +171,6 @@ def non_max_suppression( boxes_xyxy = xywh2xyxy(x[:, :4]) scores_per_class = x[:, 4:] # [N, K] - K = scores_per_class.shape[1] # Best class per box cls_conf, cls_idx = scores_per_class.max(dim=1) diff --git a/sparrow/xbee_configure.py b/sparrow/xbee_configure.py index a6411e3..59ac84e 100644 --- a/sparrow/xbee_configure.py +++ b/sparrow/xbee_configure.py @@ -79,7 +79,8 @@ def configure_xbee( try: if ser: ser.close() - except Exception: + except (serial.SerialException, OSError): + # Port is already unhealthy; drop it and keep probing. pass ser = None diff --git a/sparrow/xbee_master_collect.py b/sparrow/xbee_master_collect.py index 41c0831..d9122e6 100644 --- a/sparrow/xbee_master_collect.py +++ b/sparrow/xbee_master_collect.py @@ -134,12 +134,14 @@ def open_serial_forever(port: str, baud: int, logger: logging.Logger, timeout: f try: ser.reset_input_buffer() - except Exception: + except (serial.SerialException, OSError): + # Buffer reset is best-effort on freshly-opened ports; ignore. pass try: ser.reset_output_buffer() - except Exception: + except (serial.SerialException, OSError): + # Buffer reset is best-effort on freshly-opened ports; ignore. pass time.sleep(0.5) @@ -224,14 +226,14 @@ def main(): # File-type routing. Robin sends `.jpg` images alongside `.mp3` audio over the # same XBee link; we split them at save time so each downstream pipeline # (inference.py for images, rest_client for audio) finds only what it expects. - IMAGE_EXTS = (".jpg", ".jpeg", ".png") AUDIO_EXTS = (".mp3", ".wav") def out_dir_for(name: str) -> str: ext = os.path.splitext(name)[1].lower() if ext in AUDIO_EXTS: return audio_out - # Unknown extensions land with images so they're visible and not silently dropped. + # Image and unknown extensions land in args.out so they're visible + # to inference.py and not silently dropped. return args.out dev_queues: dict[int, deque] = {} @@ -250,14 +252,14 @@ def next_frame_id(cur: int) -> int: return (cur % 255) + 1 def send_to(src64_int: int, rf: bytes): - nonlocal frame_id, ser + nonlocal frame_id tx = build_tx_request_0x10(frame_id, src64_int, rf) ser.write(tx) ser.flush() frame_id = next_frame_id(frame_id) def reset_link_state(reason: str): - nonlocal active, sessions, master_id_logged + nonlocal active, master_id_logged if active is not None or sessions: logger.warning( "LINK reset reason=%s clearing active=%s partial_sessions=%d", @@ -270,7 +272,7 @@ def reset_link_state(reason: str): master_id_logged = False def log_local_xbee_identity(): - nonlocal master_id_logged, ser + nonlocal master_id_logged if master_id_logged or ser is None: return @@ -427,7 +429,8 @@ def requeue_active_as_ready(): logger.warning("No serial frames for %.1fs; reopening port", args.serial_idle_reopen) try: ser.close() - except Exception: + except (serial.SerialException, OSError): + # Port is already unhealthy; best-effort close before reopen. pass ser = None reset_link_state("serial-idle") @@ -590,7 +593,8 @@ def requeue_active_as_ready(): try: if ser is not None: ser.close() - except Exception: + except (serial.SerialException, OSError): + # Port is already unhealthy; best-effort close before reopen. pass ser = None reset_link_state("serial-fault") @@ -601,7 +605,8 @@ def requeue_active_as_ready(): try: if ser is not None: ser.close() - except Exception: + except (serial.SerialException, OSError): + # Port is already unhealthy; best-effort close before reopen. pass ser = None reset_link_state("unhandled-exception") @@ -613,7 +618,8 @@ def requeue_active_as_ready(): try: if ser is not None: ser.close() - except Exception: + except (serial.SerialException, OSError): + # Shutdown cleanup; best-effort. pass