Skip to content

Conversation

@matteius
Copy link
Contributor

No description provided.

@matteius matteius requested a review from Copilot June 28, 2025 13:52
@matteius matteius marked this pull request as ready for review June 28, 2025 13:52
Copy link

Copilot AI left a 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_tracker module 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.py to 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}")
Copy link

Copilot AI Jun 28, 2025

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.

Suggested change
print(f"Recorded pump activity: {activity.description}")
logging.info(f"Recorded pump activity: {activity.description}")

Copilot uses AI. Check for mistakes.
}

# Calculate per-pump statistics
for position in range(1, 9): # Pumps 1-8
Copy link

Copilot AI Jun 28, 2025

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.

Suggested change
for position in range(1, 9): # Pumps 1-8
for position in range(1, NUM_PUMPS + 1): # Iterate over all pumps

Copilot uses AI. Check for mistakes.
# Clear pump activities after successful report
clear_reported_activities()
except Exception as e:
print(f"Error reporting to API: {e}")
Copy link

Copilot AI Jun 28, 2025

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.

Suggested change
print(f"Error reporting to API: {e}")
logging.error(f"Error reporting to API: {e}", exc_info=True)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants