-
Notifications
You must be signed in to change notification settings - Fork 2
Track pump activity #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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}") |
Copilot
AI
Jun 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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 |
Copilot
AI
Jun 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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}") |
Copilot
AI
Jun 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.