Merge pull request #19 from CommonplaceRobotics/pre-commit

Pre commit Github actions from @michaelosthege

Author: cpr-bar

.github/workflows/pre-commit.yml

@@ -0,0 +1,16 @@
+name: pre-commit
+
+on:
+  pull_request:
+  push:
+    branches: [master, pre-commit]
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions/setup-python@v5
+      with:
+        python-version: '3.x'
+    - uses: pre-commit/[email protected]

.github/workflows/tests.yml

@@ -0,0 +1,26 @@
+name: tests
+
+on:
+  pull_request:
+  push:
+    branches: [master]
+
+jobs:
+  test-job:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["3.10", "3.11", "3.12"]
+    steps:
+    - uses: actions/checkout@v4
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v5
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Install dependencies
+      run: |
+        pip install -e .
+        pip install -r requirements.txt
+    - name: Test with pytest
+      run: |
+        pytest --cov=./cri_lib --cov-report term-missing tests

.pre-commit-config.yaml

@@ -0,0 +1,29 @@
+exclude: \.(html|js)$
+repos:
+- repo: https://github.com/pre-commit/pre-commit-hooks
+  rev: v5.0.0
+  hooks:
+    - id: check-merge-conflict
+    - id: check-toml
+    - id: check-yaml
+    - id: end-of-file-fixer
+    - id: trailing-whitespace
+- repo: https://github.com/pre-commit/mirrors-mypy
+  rev: v1.15.0
+  hooks:
+  -   id: mypy
+      exclude: 'test_.*?\.py$'
+- repo: https://github.com/astral-sh/ruff-pre-commit
+  rev: v0.10.0
+  hooks:
+    # Run the formatter.
+    - id: ruff-format
+- repo: https://github.com/psf/black
+  rev: 25.1.0
+  hooks:
+    - id: black
+- repo: https://github.com/PyCQA/isort
+  rev: 6.0.1
+  hooks:
+    - id: isort
+      name: isort

README.md

@@ -1,4 +1,4 @@
-# Introduction 
+# Introduction
 Python package to interface an igus Robot Control via the CRI protocol.
 
 # Current Features
@@ -8,7 +8,7 @@ Python package to interface an igus Robot Control via the CRI protocol.
     - Enable / Disable
     - Acquisition of active control
     - Override
-- Referencing of 
+- Referencing of
     - single axis
     - all axes
     - Set joints to zero
@@ -50,5 +50,5 @@ The robot state is continuously sent to the computer by the iRC/ReBeL. It gets r
 See `examples` directory.
 
 # Tests
-This repository provides pytests for the message parser. They require `pytest` and `pytest-cov`, which can be installed via `pip install -r requirements.txt`. To run the tests (including coverage) execute the following command: `pytest -vv --cov=cri_lib --cov-report term-missing tests`. 
-
+This repository provides pytests for the message parser. They require `pytest` and `pytest-cov`, which can be installed via `pip install -r requirements.txt`. To run the tests (including coverage) execute the following command: `pytest -vv --cov=cri_lib --cov-report term-missing tests`.
+Runs as pre-commit Github action.

cri_lib/__init__.py

@@ -2,23 +2,22 @@
 .. include:: ../README.md
 """
 
+from .cri_controller import CRIController
+from .cri_errors import CRICommandTimeOutError, CRIConnectionError, CRIError
+from .cri_protocol_parser import CRIProtocolParser
 from .robot_state import (
-    RobotMode,
+    ErrorStates,
+    JointsState,
     KinematicsState,
+    OperationInfo,
     OperationMode,
-    RunState,
-    ReplayMode,
-    ErrorStates,
-    RobotCartesianPosition,
     PlatformCartesianPosition,
-    JointsState,
-    RobotState,
     PosVariable,
-    OperationInfo,
     ReferencingAxisState,
     ReferencingState,
+    ReplayMode,
+    RobotCartesianPosition,
+    RobotMode,
+    RobotState,
+    RunState,
 )
-from .cri_controller import CRIController
-from .cri_protocol_parser import CRIProtocolParser
-
-from .cri_errors import CRIError, CRIConnectionError, CRICommandTimeOutError

cri_lib/cri_controller.py

@@ -1,17 +1,15 @@
-from enum import Enum
 import logging
 import socket
 import threading
-from time import sleep, time
+from enum import Enum
 from pathlib import Path
-from typing import Callable
-from queue import Queue, Empty
-
-from .robot_state import RobotState, KinematicsState
-
-from .cri_protocol_parser import CRIProtocolParser
+from queue import Empty, Queue
+from time import sleep, time
+from typing import Any, Callable
 
 from .cri_errors import CRICommandTimeOutError, CRIConnectionError
+from .cri_protocol_parser import CRIProtocolParser
+from .robot_state import KinematicsState, RobotState
 
 logger = logging.getLogger(__name__)
 
@@ -34,14 +32,14 @@ class MotionType(Enum):
         CartTool = "CartTool"
         Platform = "Platform"
 
-    def __init__(self):
+    def __init__(self) -> None:
         self.robot_state: RobotState = RobotState()
         self.robot_state_lock = threading.Lock()
 
         self.parser = CRIProtocolParser(self.robot_state, self.robot_state_lock)
 
         self.connected = False
-        self.sock = None
+        self.sock: socket.socket | None = None
         self.socket_write_lock = threading.Lock()
 
         self.can_mode: bool = False
@@ -58,7 +56,7 @@ def __init__(self):
         self.answer_events: dict[str, threading.Event] = {}
         self.error_messages: dict[str, str] = {}
 
-        self.status_callback = None
+        self.status_callback: Callable | None = None
 
         self.live_jog_active: bool = False
         self.jog_intervall = self.ALIVE_JOG_INTERVAL_SEC
@@ -125,7 +123,7 @@ def close(self) -> None:
         Close network connection. Might block for a while waiting for the threads to finish.
         """
 
-        if not self.connected:
+        if not self.connected or self.sock is None:
             return
 
         self._send_command("QUIT")
@@ -163,7 +161,7 @@ def _send_command(
             If the command was sent the message_id gets returned or None if there was an error.
 
         """
-        if not self.connected:
+        if not self.connected or self.sock is None:
             logger.error("Not connected. Use connect() to establish a connection.")
             raise CRIConnectionError(
                 "Not connected. Use connect() to establish a connection."
@@ -208,7 +206,6 @@ def _bg_alivejog_thread(self) -> None:
         """
         Background Thread sending alivejog messages to keep connection alive.
         """
-
         while self.connected:
             if self.live_jog_active:
                 with self.jog_speeds_lock:
@@ -227,6 +224,10 @@ def _bg_receive_thread(self) -> None:
         """
         Background thread receiving data and parsing it to the robot state.
         """
+        if self.sock is None:
+            logger.error("Receive Thread: Not connected.")
+            return
+
         message_buffer = bytearray()
 
         while self.connected:
@@ -291,7 +292,7 @@ def _wait_for_answer(
 
         with self.answer_events_lock:
             if message_id not in self.answer_events:
-                return False
+                return None
             wait_event = self.answer_events[message_id]
 
         # prevent deadlock through answer_events_lock
@@ -326,8 +327,8 @@ def _parse_message(self, message: str) -> None:
                 msg_id = notification["answer"]
 
                 if msg_id in self.answer_events:
-                    if "error" in notification:
-                        self.error_messages[msg_id] = notification["error"]
+                    if (error_msg := notification.get("error", None)) is not None:
+                        self.error_messages[msg_id] = error_msg
 
                     self.answer_events[msg_id].set()
 
@@ -428,7 +429,7 @@ def disable(self) -> bool:
         else:
             return False
 
-    def set_active_control(self, active: bool) -> None:
+    def set_active_control(self, active: bool) -> bool:
         """Acquire or return active control of robot
 
         Parameters
@@ -480,7 +481,7 @@ def zero_all_joints(self) -> bool:
         else:
             return False
 
-    def reference_all_joints(self) -> bool:
+    def reference_all_joints(self, *, timeout: float = 30) -> bool:
         """Reference all joints. Long timout of 30 seconds.
 
         Returns
@@ -492,7 +493,7 @@ def reference_all_joints(self) -> bool:
 
         if (msg_id := self._send_command("CMD ReferenceAllJoints", True)) is not None:
             if (
-                error_msg := self._wait_for_answer(f"{msg_id}", timeout=30.0)
+                error_msg := self._wait_for_answer(f"{msg_id}", timeout=timeout)
             ) is not None:
                 logger.debug("Error in ReferenceAllJoints command: %s", error_msg)
                 return False
@@ -501,7 +502,7 @@ def reference_all_joints(self) -> bool:
         else:
             return False
 
-    def reference_single_joint(self, joint: str) -> bool:
+    def reference_single_joint(self, joint: str, *, timeout: float = 30) -> bool:
         """Reference a single joint. Long timout of 30 seconds.
 
         Parameters
@@ -527,7 +528,7 @@ def reference_single_joint(self, joint: str) -> bool:
             msg_id := self._send_command(f"CMD ReferenceSingleJoint {joint_msg}", True)
         ) is not None:
             if (
-                error_msg := self._wait_for_answer(f"{msg_id}", timeout=30.0)
+                error_msg := self._wait_for_answer(f"{msg_id}", timeout=timeout)
             ) is not None:
                 logger.debug("Error in ReferenceSingleJoint command: %s", error_msg)
                 return False
@@ -561,7 +562,7 @@ def get_referencing_info(self):
         else:
             return False
 
-    def wait_for_kinematics_ready(self, timeout: float | None = 30) -> bool:
+    def wait_for_kinematics_ready(self, timeout: float = 30) -> bool:
         """Wait until drive state is indicated as ready.
 
         Parameters
@@ -955,7 +956,7 @@ def move_tool_relative(
         else:
             return False
 
-    def stop_move(self) -> None:
+    def stop_move(self) -> bool:
         """Stop movement
 
         Returns
@@ -1374,6 +1375,7 @@ def upload_file(self, path: str | Path, target_directory: str) -> bool:
 
         if self._send_command(command, True) is None:
             return False
+        return True
 
     def enable_can_bridge(self, enabled: bool) -> None:
         """Enables or diables CAN bridge mode. All other functions are disabled in CAN bridge mode.
@@ -1415,7 +1417,7 @@ def can_send(self, msg_id: int, length: int, data: bytearray) -> None:
 
     def can_receive(
         self, blocking: bool = True, timeout: float | None = None
-    ) -> dict[str, any] | None:
+    ) -> dict[str, Any] | None:
         """Receive CAN message in CAN bridge mode from the recveive queue.
 
         Returns
@@ -1425,7 +1427,7 @@ def can_receive(
         """
         if not self.can_mode:
             logger.debug("can_receive: CAN mode not enabled")
-            return
+            return None
 
         try:
             item = self.can_queue.get(blocking, timeout)

cri_lib/cri_errors.py

@@ -1,14 +1,18 @@
 class CRIError(Exception):
     """Base class for CRI-related errors."""
 
+
 class CRIConnectionError(CRIError):
     """Raised when there is a connection error."""
+
     def __init__(self, message="Not connected to iRC or connection lost."):
         self.message = message
         super().__init__(self.message)
 
+
 class CRICommandTimeOutError(CRIError):
     """Raised when a command times out."""
+
     def __init__(self, message="Time out waiting for command response."):
         self.message = message
-        super().__init__(self.message)
+        super().__init__(self.message)