Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds tracking of pump dosing events throughout a GrowMax run and includes that data in the environment report to the API.
- Introduce a new
pump_trackermodule to record, retrieve, and clear pump activities. - Update the main routine (
routine.py) to include pump activities in the API payload and clear them on successful reporting. - Modify
pump.pyto record each dosing event via the tracker.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/growmax/routine.py | Collect and include pump activities in the API report; wrap reporting in try/except and clear on success. |
| src/growmax/pump_tracker.py | New module defining PumpActivity, PumpTracker, and convenience functions for session and statistics. |
| src/growmax/pump.py | Store channel in the pump and call record_pump_dose during dosing. |
Comments suppressed due to low confidence (2)
src/growmax/pump_tracker.py:1
- The new pump_tracker module lacks accompanying unit tests; consider adding tests for record_pump_activity, get_session_activities, and get_pump_statistics.
"""
src/growmax/routine.py:124
- [nitpick] Nesting the key 'pumps' inside another 'pumps' object may be redundant; consider flattening so consumers receive a list directly (e.g., report_data['pumps'] = pump_activities).
report_data["pumps"] = {
| if len(self.activities) > self.max_activities: | ||
| self.activities = self.activities[-self.max_activities:] | ||
|
|
||
| print(f"Recorded pump activity: {activity.description}") |
There was a problem hiding this comment.
[nitpick] Using print for production logging can clutter stdout; consider using the logging module at a debug or info level instead.
| print(f"Recorded pump activity: {activity.description}") | |
| logging.info(f"Recorded pump activity: {activity.description}") |
| } | ||
|
|
||
| # Calculate per-pump statistics | ||
| for position in range(1, 9): # Pumps 1-8 |
There was a problem hiding this comment.
[nitpick] Hard-coded pump count; consider using a named constant or configuration value instead of a magic number to support changes in pump capacity.
| for position in range(1, 9): # Pumps 1-8 | |
| for position in range(1, NUM_PUMPS + 1): # Iterate over all pumps |
| # Clear pump activities after successful report | ||
| clear_reported_activities() | ||
| except Exception as e: | ||
| print(f"Error reporting to API: {e}") |
There was a problem hiding this comment.
[nitpick] Consider using structured logging (e.g., logging.error) instead of print to capture error context and stack traces in production environments.
| print(f"Error reporting to API: {e}") | |
| logging.error(f"Error reporting to API: {e}", exc_info=True) |
No description provided.