Updates for SR2025

.github/workflows/test_build.yml

@@ -1,30 +1,32 @@
 name: Lint & build
 
-on: [push]
+on:
+  push:
+  workflow_dispatch:
 
 jobs:
   test:
     strategy:
       fail-fast: false
       matrix:
         os: [ubuntu-latest]
-        py_version: ["3.8", "3.9", "3.10", "3.11"]
+        py_version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
         include:
           - os: windows-latest
             py_version: "3.8"
           - os: windows-latest
-            py_version: "3.11"
+            py_version: "3.12"
           - os: macos-latest
             py_version: "3.8"
           - os: macos-latest
-            py_version: "3.11"
+            py_version: "3.12"
     runs-on: ${{ matrix.os }}
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
       with:
         fetch-depth: 0
     - name: Set up Python
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.py_version }}
     - name: Install dependencies
@@ -50,11 +52,11 @@ jobs:
     runs-on: ubuntu-latest
     needs: test
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
       with:
         fetch-depth: 0
     - name: Set up Python 3.8
-      uses: actions/setup-python@v4
+      uses: actions/setup-python@v5
       with:
         python-version: "3.8"
     - name: Install dependencies
@@ -66,13 +68,11 @@ jobs:
       run: |
         make build
     - name: Save built package
-      uses: actions/upload-artifact@v3
+      uses: actions/upload-artifact@v4
       with:
         name: package
         path: |
           dist
     - name: Publish to PyPi
       if: ${{ github.ref_type	== 'tag' }}
       uses: pypa/gh-action-pypi-publish@release/v1
-      with:
-        print_hash: true

pyproject.toml

@@ -54,7 +54,7 @@ requires-python = ">=3.8"
 dependencies = [
     "pyserial >=3,<4",
     "april_vision >=2.1,<3",
-    "paho-mqtt >=1.6,<2",
+    "paho-mqtt >=2,<3",
     "pydantic >=1.9.1,<2",
     "typing-extensions; python_version<'3.10'",
 ]
@@ -75,11 +75,12 @@ Documentation = "https://docs.studentrobotics.org"
 dev = [
     "flake8",
     "isort",
-    "mypy",
+    "mypy==1.10.0; python_version<'3.9'",
+    "mypy>=1.7,<2; python_version>='3.9'",
     "build",
     "types-pyserial",
     "pytest",
     "pytest-cov",
-    "types-paho-mqtt",
+    "paho-mqtt >=2,<3"
 ]
 vision = ["opencv-python-headless >=4,<5"]

sr/robot3/astoria.py

@@ -9,8 +9,9 @@
 from time import sleep
 from typing import Any, ClassVar, NewType, Optional, Tuple
 
+import paho.mqtt.client as mqtt
 from paho.mqtt.client import Client as MQTT
-from paho.mqtt.client import MQTTMessage, MQTTv5, MQTTv311
+from paho.mqtt.client import MQTTMessage
 from pydantic import BaseModel, ValidationError
 
 from .mqtt import MQTTClient
@@ -278,7 +279,11 @@ def init_mqtt(config: AstoriaConfig, client_name: str = 'sr-robot3') -> 'MQTTCli
         host=config.mqtt.host,
         port=config.mqtt.port,
         client_name=client_name,
-        mqtt_version=MQTTv311 if config.mqtt.force_protocol_version_3_1 else MQTTv5,
+        mqtt_version=(
+            mqtt.MQTTProtocolVersion.MQTTv311
+            if config.mqtt.force_protocol_version_3_1 else
+            mqtt.MQTTProtocolVersion.MQTTv5
+        ),
         topic_prefix=config.mqtt.topic_prefix,
     )
     return client

sr/robot3/mqtt.py

@@ -3,6 +3,7 @@
 import atexit
 import json
 import logging
+import time
 from threading import Lock
 from typing import Any, Callable, Optional, TypedDict, Union
 from urllib.parse import urlparse
@@ -17,7 +18,7 @@ def __init__(
         self,
         client_name: Optional[str] = None,
         topic_prefix: Optional[str] = None,
-        mqtt_version: int = mqtt.MQTTv5,
+        mqtt_version: mqtt.MQTTProtocolVersion = mqtt.MQTTProtocolVersion.MQTTv5,
         use_tls: Union[bool, str] = False,
         username: str = '',
         password: str = '',
@@ -29,7 +30,11 @@ def __init__(
         self.topic_prefix = topic_prefix
         self._client_name = client_name
 
-        self._client = mqtt.Client(client_id=client_name, protocol=mqtt_version)
+        self._client = mqtt.Client(
+            callback_api_version=mqtt.CallbackAPIVersion.VERSION2,
+            client_id=client_name,
+            protocol=mqtt_version,
+        )
         self._client.on_connect = self._on_connect
 
         if use_tls:
@@ -143,19 +148,29 @@ def wrapped_publish(
         """Wrap a payload up to be decodable as JSON."""
         if isinstance(payload, bytes):
             payload = payload.decode('utf-8')
+
+        payload_dict = {
+            "timestamp": time.time(),
+            "data": payload,
+        }
+
         self.publish(
             topic,
-            json.dumps({"data": payload}),
+            json.dumps(payload_dict),
             retain=retain, abs_topic=abs_topic)
 
     def _on_connect(
-        self, client: mqtt.Client, userdata: Any, flags: dict[str, int], rc: int,
-        properties: Optional[mqtt.Properties] = None,
+        self,
+        client: mqtt.Client,
+        userdata: Any,
+        connect_flags: mqtt.ConnectFlags,
+        reason_code: mqtt.ReasonCode,
+        properties: mqtt.Properties | None = None,
     ) -> None:
         """Callback run each time the client connects to the broker."""
-        if rc != mqtt.CONNACK_ACCEPTED:
+        if reason_code.is_failure:
             LOGGER.warning(
-                f"Failed to connect to MQTT broker. Return code: {mqtt.error_string(rc)}"
+                f"Failed to connect to MQTT broker. Return code: {reason_code.getName()}"  # type: ignore[no-untyped-call] # noqa: E501
             )
             return
 
@@ -182,7 +197,7 @@ def unpack_mqtt_url(url: str) -> MQTTVariables:
     The URL should be in the format:
     mqtt[s]://[<username>[:<password>]]@<host>[:<port>]/<topic_root>
     """
-    url_parts = urlparse(url)
+    url_parts = urlparse(url, allow_fragments=False)
 
     if url_parts.scheme not in ('mqtt', 'mqtts'):
         raise ValueError(f"Invalid scheme: {url_parts.scheme}")

sr/robot3/power_board.py

@@ -501,7 +501,7 @@ def buzz(self, frequency: float, duration: float, *, blocking: bool = False) ->
             frequency, 8, 10_000, "Frequency must be between 8 and 10000Hz"))
         duration_ms = int(float_bounds_check(
             duration * 1000, 0, 2**31 - 1,
-            f"Duration is a float in seconds between 0 and {(2**31-1)/1000:,.0f}"))
+            f"Duration is a float in seconds between 0 and {(2**31 - 1) / 1000:,.0f}"))
 
         cmd = f'NOTE:{frequency_int}:{duration_ms}'
         self._serial.write(cmd)

sr/robot3/serial_wrapper.py

@@ -29,6 +29,7 @@
 RetType = TypeVar("RetType")
 
 E = TypeVar("E", bound=BaseException)
+BASE_TIMEOUT: float | None = 0.5
 
 
 def retry(
@@ -80,7 +81,7 @@ def __init__(
         self,
         port: str,
         baud: int,
-        timeout: float = 0.5,
+        timeout: float | None = BASE_TIMEOUT,
         identity: BoardIdentity = BoardIdentity(),
         delay_after_connect: float = 0,
     ):
@@ -96,6 +97,7 @@ def __init__(
             port,
             baudrate=baud,
             timeout=timeout,
+            write_timeout=timeout,
             do_not_open=True,
         )
 

sr/robot3/servo_board.py

@@ -20,6 +20,7 @@
 DUTY_MAX = 4000
 START_DUTY_MIN = 1000
 START_DUTY_MAX = 2000
+NUM_SERVOS = 12
 
 logger = logging.getLogger(__name__)
 BAUDRATE = 115200  # Since the servo board is a USB device, this is ignored
@@ -77,7 +78,7 @@ def __init__(
         self._serial = SerialWrapper(serial_port, BAUDRATE, identity=initial_identity)
 
         self._servos = tuple(
-            Servo(self._serial, index) for index in range(12)
+            Servo(self._serial, index) for index in range(NUM_SERVOS)
         )
 
         self._identity = self.identify()

sr/robot3/timeout.py

@@ -1,4 +1,5 @@
 """Functions for killing the robot after a certain amount of time."""
+import atexit
 import logging
 import os
 import signal
@@ -9,6 +10,9 @@
 
 logger = logging.getLogger(__name__)
 
+TIMEOUT_MESSAGE = "Timeout expired: Game Over!"
+EXIT_ATTEMPTS = 0
+
 
 def timeout_handler(signal_type: int, stack_frame: Optional[FrameType]) -> None:
     """
@@ -17,13 +21,34 @@ def timeout_handler(signal_type: int, stack_frame: Optional[FrameType]) -> None:
     This function is called when the timeout expires and will stop the robot's main process.
     In order for this to work, any threads that are created must be daemons.
 
+    If the process doesn't terminate clearly (perhaps because the exception was caught),
+    exit less cleanly.
+
     NOTE: This function is not called on Windows.
 
     :param signal_type: The signal that triggered this handler
     :param stack_frame: The stack frame at the time of the signal
     """
-    logger.info("Timeout expired: Game Over!")
-    exit(0)
+    global EXIT_ATTEMPTS
+    EXIT_ATTEMPTS += 1
+
+    if sys.platform == "win32":
+        raise AssertionError("This function should not be called on Windows")
+
+    if EXIT_ATTEMPTS == 1:
+        # Allow 2 seconds for the process to exit cleanly before killing it
+        signal.alarm(2)
+        logger.info(TIMEOUT_MESSAGE)
+        # Exit cleanly
+        exit(0)
+    else:
+        # The process didn't exit cleanly, so first call the cleanup handlers
+        # and use an unhanded alarm to force python to exit with a core dump
+        signal.signal(signal.SIGALRM, signal.SIG_DFL)
+        signal.alarm(2)  # Allow 2 seconds for cleanup
+
+        atexit._run_exitfuncs()
+        exit(0)
 
 
 def win_timeout_handler() -> None:
@@ -35,7 +60,7 @@ def win_timeout_handler() -> None:
 
     NOTE: This function is only called on Windows.
     """
-    logger.info("Timeout expired: Game Over!")
+    logger.info(TIMEOUT_MESSAGE)
     os.kill(os.getpid(), signal.SIGTERM)
 
 
@@ -52,6 +77,7 @@ def kill_after_delay(timeout_seconds: int) -> None:
         # Windows doesn't have SIGALRM,
         # so we approximate its functionality using a delayed SIGTERM
         timer = Timer(timeout_seconds, win_timeout_handler)
+        timer.daemon = True
         timer.start()
     else:
         signal.signal(signal.SIGALRM, timeout_handler)

tests/conftest.py

@@ -66,7 +66,7 @@ def query(self, request: Optional[str], endl: str = '\n') -> str:
         """
         # Assert that we have not run out of responses
         # and that the request is the next one we expect
-        assert self.request_index < len(self.responses)
+        assert self.request_index < len(self.responses), f"Unexpected request: {request}"
         assert request == self.responses[self.request_index][0]
 
         # Fetch the response and increment the request index

tests/test_data/timeout_scripts/catch-base-exception.py

@@ -0,0 +1,10 @@
+from sr.robot3.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+while True:
+    try:
+        time.sleep(10)
+    except BaseException:
+        pass

tests/test_data/timeout_scripts/catch-exception.py

@@ -0,0 +1,10 @@
+from sr.robot3.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+while True:
+    try:
+        time.sleep(10)
+    except Exception:
+        pass

tests/test_data/timeout_scripts/hot-loop.py

@@ -0,0 +1,7 @@
+from sr.robot3.timeout import kill_after_delay
+
+kill_after_delay(2)
+
+# This is a bad idea
+while True:
+    pass

tests/test_data/timeout_scripts/sleep.py

@@ -0,0 +1,6 @@
+from sr.robot3.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+time.sleep(10)

tests/test_data/timeout_scripts/try-finally.py

@@ -0,0 +1,11 @@
+from sr.robot3.timeout import kill_after_delay
+import time
+
+kill_after_delay(2)
+
+while True:
+    try:
+        time.sleep(10)
+    finally:
+        # Imagine we wanted to do something else here
+        pass

tests/test_data/timeout_scripts_extra/early-exit.py

@@ -0,0 +1,3 @@
+from sr.robot3.timeout import kill_after_delay
+
+kill_after_delay(2)

tests/test_data/timeout_scripts_extra/exception.py

@@ -0,0 +1,5 @@
+from sr.robot3.timeout import kill_after_delay
+
+kill_after_delay(2)
+
+1 / 0

tests/test_power_board.py

@@ -195,7 +195,7 @@ def test_power_board_outputs(powerboard_serial: MockPowerBoard) -> None:
     power_board.outputs[PowerOutputPosition.FIVE_VOLT].is_enabled = True
     power_board.outputs[PowerOutputPosition.FIVE_VOLT].is_enabled = False
 
-    # Test that we can't enable or disable the 5V output
+    # Test that we can't enable or disable the brain output
     with pytest.raises(RuntimeError, match=r"Brain output cannot be controlled.*"):
         power_board.outputs[PowerOutputPosition.L2].is_enabled = True