ridesx: power off device if power is referenced#716
ridesx: power off device if power is referenced#716mangelajo merged 2 commits intojumpstarter-dev:mainfrom
Conversation
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdds post-flash cleanup logic to the RideSX flash flow: after successful flash completion, the device power is turned off if a power child component exists, with corresponding logging; otherwise logs that the device remains running. Minor syntax adjustment adds trailing comma to driver initialization argument. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as RideSXClient
participant Power as PowerChild
participant Log as Logger
User->>Client: flash()
activate Client
Client->>Client: perform flash
Client->>Client: check power child exists
alt Power child exists
Client->>Power: power_off()
activate Power
Power-->>Client: ack
deactivate Power
Client->>Log: log power-off event
else No power child
Client->>Log: log device remains running
end
Client-->>User: flash complete
deactivate Client
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py (1)
102-107: Consider adding error handling for power-off operation.If
self.power.off()raises an exception, the entire flash operation will fail even though the flash itself completed successfully. This could confuse users and make debugging harder.Consider wrapping the power-off call in a try-except block:
self.logger.info("flash operation completed successfully") if "power" in self.children: - self.power.off() - self.logger.info("device powered off") + try: + self.power.off() + self.logger.info("device powered off") + except Exception as e: + self.logger.warning(f"failed to power off device: {e}") else: self.logger.info("device left running")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py(1 hunks)packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py (1)
packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py (4)
off(16-18)off(50-52)off(65-66)off(73-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (1)
packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py (1)
102-107: Verify the timing between fastboot continue and power-off.The flash process ends with a
fastboot continuecommand (inflash_with_fastbootat line 186 of driver.py) which instructs the device to exit fastboot mode and boot normally. Immediately after, this code powers off the device.Ensure this immediate power-off doesn't interfere with the device's boot process or leave it in an inconsistent state.
If this behavior is intentional (e.g., to prevent auto-boot into the new image), consider adding a brief comment explaining the rationale. Otherwise, you might want to add a small delay or check device state before powering off.
|
Successfully created backport PR for |
2 participants
Summary by CodeRabbit