Skip to content

Movements#5

Merged
sgbaird merged 10 commits intomainfrom
movements
Nov 19, 2025
Merged

Movements#5
sgbaird merged 10 commits intomainfrom
movements

Conversation

@sgbaird
Copy link
Copy Markdown
Member

@sgbaird sgbaird commented Nov 19, 2025

merged in wrong order originally

Copilot AI and others added 10 commits November 18, 2025 21:50
…prehensive documentation

Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
…ect CLI usage

Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
…loyment_examples

Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
…logic, extract testing functions

Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
…ing to tests folder

Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
Co-authored-by: sgbaird <45469701+sgbaird@users.noreply.github.com>
Add Prefect-based async deployment for desk lifter control
Copilot AI review requested due to automatic review settings November 19, 2025 01:38
@sgbaird sgbaird merged commit f96b3a4 into main Nov 19, 2025
5 checks passed
Copy link
Copy Markdown

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 comprehensive desk lifter control functionality to the progressive-automations-python package, introducing Prefect workflow orchestration, GPIO-based movement control, duty cycle management, and testing utilities. The changes enable remote desk control via Prefect Cloud with safety features to protect the motor.

Key changes:

  • Adds Prefect workflow orchestration with async deployment and polling capabilities
  • Implements GPIO-based movement control and duty cycle management for motor protection
  • Provides CLI tools for testing and monitoring

Reviewed Changes

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

Show a summary per file
File Description
tests/testing.py Testing utilities for movement sequences and configuration-based execution
src/progressive_automations_python/prefect_flows.py Prefect flow definitions and deployment functions
src/progressive_automations_python/movement_control.py Low-level GPIO control for desk movements
src/progressive_automations_python/duty_cycle.py Duty cycle tracking and enforcement logic
src/progressive_automations_python/desk_controller.py High-level desk controller with safety checks
src/progressive_automations_python/deployment.py Prefect deployment configuration utilities
src/progressive_automations_python/config.py Configuration settings with validation
src/progressive_automations_python/cli.py Command-line interface for testing
src/progressive_automations_python/init.py Package exports and API definitions
setup.cfg Dependencies and console script entry points
docs/quick-start.md Quick start guide for setup and usage
docs/installation-and-usage.md Comprehensive installation and usage documentation
docs/index.md Documentation index updates
README.md Updated package description and usage examples

)

from progressive_automations_python.prefect_flows import (
simple_movement_flow,
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate export of simple_movement_flow in __all__ list. The second occurrence should likely be custom_movements_flow, test_sequence_flow, duty_cycle_monitoring_flow, or scheduled_duty_cycle_check which are imported but not exported.

Copilot uses AI. Check for mistakes.
Comment on lines +83 to +85
deployment = custom_movements_flow.from_source(
source=".",
entrypoint="prefect_flows.py:custom_movements_flow",
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

References undefined custom_movements_flow. This flow is not defined in the new prefect_flows.py file, but is referenced in deployment functions and imports in init.py. The flow definition is missing from this file.

Copilot uses AI. Check for mistakes.

if len(sys.argv) > 1:
if sys.argv[1] == "test":
test_sequence_flow()
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

References undefined test_sequence_flow. This flow is not defined in the new prefect_flows.py file but is called in the __main__ section. The flow definition is missing.

Copilot uses AI. Check for mistakes.
if sys.argv[1] == "test":
test_sequence_flow()
elif sys.argv[1] == "movements":
custom_movements_flow()
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

References undefined custom_movements_flow. This flow is not defined in the new prefect_flows.py file but is called in the __main__ section. The flow definition is missing.

Copilot uses AI. Check for mistakes.
elif sys.argv[1] == "movements":
custom_movements_flow()
elif sys.argv[1] == "monitor":
duty_cycle_monitoring_flow()
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

References undefined duty_cycle_monitoring_flow. This flow is not defined in the new prefect_flows.py file but is called in the __main__ section. The flow definition is missing.

Copilot uses AI. Check for mistakes.

# Phase 2: Rest
print(f"\n--- Phase 2: Resting for {rest_time} seconds ---")
import time
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import statement placed in the middle of function body. The time module should be imported at the top of the file with other imports to follow Python conventions.

Copilot uses AI. Check for mistakes.
logger.warning(f"⏳ Duty cycle capacity insufficient - will wait {wait_time:.1f}s before movement")

# Wait for duty cycle to free up
import time
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import statement placed in the middle of function body. The time module should be imported at the top of the file with other imports to follow Python conventions.

Copilot uses AI. Check for mistakes.
Comment on lines +58 to +66









Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excessive blank lines (9 consecutive blank lines) between functions reduce code readability. Consider removing these to maintain consistency with Python style guidelines (typically 2 blank lines between top-level functions).

Suggested change

Copilot uses AI. Check for mistakes.

The system enforces a 10% duty cycle (2 minutes on, 18 minutes off) to protect the motor:

- **Maximum continuous runtime**: 30 seconds
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation states maximum continuous runtime as 30 seconds, but config.py sets MAX_CONTINUOUS_RUNTIME = 45. This inconsistency between documentation and code could confuse users.

Copilot uses AI. Check for mistakes.
raise ValueError("No last known position in state file")

# Check movement requirements
check_result = check_movement_against_duty_cycle(target_height, current_height, UP_RATE, DOWN_RATE)
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function call passes UP_RATE and DOWN_RATE parameters, but the function signature in duty_cycle.py shows these parameters have default values of 4.8. The actual config values are 0.54 and 0.55, suggesting the defaults are incorrect and will cause movement time calculation errors.

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.

3 participants