Mergemaster

[beniroquai]

No description provided.

[Copilot]

Pull Request Overview

This pull request introduces significant enhancements to the UC2-REST framework, adding callback mechanisms for real-time status monitoring and new functionality for multi-motor homing and CAN device OTA updates.

Key Changes:

• Added callback mechanisms to laser, home, and CAN modules for real-time status monitoring
• Implemented home_xy() method for simultaneous multi-motor homing operations
• Added new CANOTA module for over-the-air firmware updates of CAN satellite devices
• Enhanced serial message parsing to support substring matching for delimiters

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
uc2rest/mserial.py	Modified delimiter detection from exact match to substring search; added TODO comment about callback list management
uc2rest/laser.py	Added numpy dependency and callback mechanism for laser status monitoring with array-based state tracking
uc2rest/home.py	Added numpy dependency, callback mechanism for homing status, and new home_xy() method for simultaneous multi-motor homing
uc2rest/canota.py	New module providing CAN OTA update functionality with WiFi configuration and callback support
uc2rest/can.py	Added callback mechanism and new methods scan() and get_available_devices() for CAN bus management
uc2rest/UC2Client.py	Integrated CANOTA module into main client
uc2rest/TEST/TEST_LASER_CALLBACK.py	Test script demonstrating laser callback functionality
uc2rest/TEST/TEST_HOME_XY.py	Test script demonstrating multi-motor homing functionality
uc2rest/TEST/TEST_CANOTA.py	Test script for CAN OTA update functionality with minor documentation inconsistencies
DOCUMENTATION/LASER_CALLBACK_UPDATE.md	Documentation for laser callback mechanism updates
DOCUMENTATION/IMPLEMENTATION_SUMMARY.md	Summary of implementation changes
DOCUMENTATION/HOME_XY_Usage.md	Usage guide for multi-motor homing
DOCUMENTATION/CAN_OTA_Documentation.md	Comprehensive documentation for CAN OTA functionality

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Debug print statement should be removed or converted to use a logger. This print statement will clutter console output in production environments. Consider using self._parent.logger.debug() instead, or removing this debug print entirely.

Suggested change

	print(self._callbackPerKey)
	self._parent.logger.debug("Callback functions initialized: %s", self._callbackPerKey)

[Copilot]

Callback slots are initialized to empty lists [] instead of None. This is inconsistent with the canota.py implementation which uses None. In the _callback_home_status method, callable() is used to check if a callback exists (line 56), but an empty list [] is callable as False, which works but is semantically incorrect. Consider initializing to None for consistency and clarity.

Suggested change

	_callbackPerKey[i] = []
	_callbackPerKey[i] = None

[Copilot]

Debug print statement should be removed or converted to use a logger. This print statement will clutter console output in production environments. Consider using self._parent.logger.debug() instead, or removing this debug print entirely.

Suggested change

	print(self._callbackPerKey)
	self._parent.logger.debug("Callback functions initialized: %s", self._callbackPerKey)

[Copilot]

Typo in comment: "tODO" should be "TODO" (with consistent capitalization).

Suggested change

	# response = ESP32.canota.start_motor_ota("X", WIFI_SSID, WIFI_PASSWORD, timeout=300000) # tODO: we should have a generic interface (e.g. by ID instead of by motor, etc. )
	# response = ESP32.canota.start_motor_ota("X", WIFI_SSID, WIFI_PASSWORD, timeout=300000) # TODO: we should have a generic interface (e.g. by ID instead of by motor, etc. )

[Copilot]

Callback slots are initialized to empty lists [] instead of None. This is inconsistent with the canota.py implementation (line 49) which uses None. In the _callback_laser_status method, callable() is used to check if a callback exists (line 56), but an empty list [] is callable as False, which works but is semantically incorrect. Consider initializing to None for consistency and clarity, as done in canota.py.

Suggested change

	_callbackPerKey[i] = []
	_callbackPerKey[i] = None

[Copilot]

Debug print statement should be removed or converted to use a logger. This print statement will clutter console output in production environments. Consider using self._parent.logger.debug() instead, or removing this debug print entirely.

Suggested change

	print(self._callbackPerKey)
	if hasattr(self._parent, "logger"):
	self._parent.logger.debug(self._callbackPerKey)

[Copilot]

Callback slots are initialized to empty lists [] instead of None. This is inconsistent with the canota.py implementation which uses None. In the _callback_can_status method, callable() is used to check if a callback exists (line 51), but an empty list [] is callable as False, which works but is semantically incorrect. Consider initializing to None for consistency and clarity.

Suggested change

	_callbackPerKey[i] = []
	_callbackPerKey[i] = None

[Copilot]

Typo in comment: "positible" should be "possible".

Suggested change

	time.sleep(10) # TODO: We should be wait for a positible callback
	time.sleep(10) # TODO: We should be wait for a possible callback

[Copilot]

The comment on line 61 says "Start OTA update on Motor X (CAN ID 11)" but the code on line 63 actually calls start_laser_ota() for laser_id=0 (CAN ID 20). This is misleading. The commented-out line 64 has the correct motor_ota call. Either uncomment line 64 and remove line 63, or update the comment on line 61 to correctly describe what the code does.

Suggested change

	response = ESP32.canota.start_laser_ota(laser_id=0, ssid=WIFI_SSID, password=WIFI_PASSWORD, timeout=300000, is_blocking=False)
	# response = ESP32.canota.start_motor_ota("X", WIFI_SSID, WIFI_PASSWORD, timeout=300000) # tODO: we should have a generic interface (e.g. by ID instead of by motor, etc. )
	response = ESP32.canota.start_motor_ota("X", WIFI_SSID, WIFI_PASSWORD, timeout=300000) # tODO: we should have a generic interface (e.g. by ID instead of by motor, etc. )

[Copilot]

Missing bounds check for stepper_id array indexing. If steppers["axis"] returns a value outside the range [0, self.nMotors-1], this will cause an IndexError. Add validation: if 0 <= stepperID < self.nMotors: before accessing self.isHomed[stepperID].

Suggested change

	self.isHomed[stepperID] = steppers["isDone"]
	elif isinstance(steppers, list):
	# Multiple steppers format: [{"axis":1,"pos":0,"isDone":1}, ...]
	for stepper in steppers:
	stepperID = stepper["axis"]
	self.isHomed[stepperID] = stepper["isDone"]
	if isinstance(stepperID, int) and 0 <= stepperID < self.nMotors:
	self.isHomed[stepperID] = steppers["isDone"]
	else:
	print(f"Warning: stepperID {stepperID} out of bounds in single stepper callback")
	elif isinstance(steppers, list):
	# Multiple steppers format: [{"axis":1,"pos":0,"isDone":1}, ...]
	for stepper in steppers:
	stepperID = stepper["axis"]
	if isinstance(stepperID, int) and 0 <= stepperID < self.nMotors:
	self.isHomed[stepperID] = stepper["isDone"]
	else:
	print(f"Warning: stepperID {stepperID} out of bounds in multi-stepper callback")