Skip to content
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

Add support for USB hub power control via uhubctl #525

Closed
wants to merge 18 commits into from

Conversation

Jckf
Copy link
Contributor

@Jckf Jckf commented Oct 15, 2022

This adds support for toggling power to certain USB hubs/ports on or off via the command line tool uhubctl (available via apt on Raspbian). This is useful to completely shut down MCUs, LCDs, and other peripherals that are powered over USB.

On my RPi 3 + RAMPS 1.4 printer setup, the following config lets me turn off the entire printer plus its webcam (port 2 controls power to all 4 ports on the RPi):

[power USB]
type: uhubctl
port: 1-1.2
bound_service: klipper

Disclaimer: I am not a Python developer, and the entire Klipper ecosystem is new to me as of a few days ago, but I've done my best 😅 Feedback welcome 🙂

Signed-off-by: Jim C K Flaten [email protected]

moonraker/components/power.py Outdated Show resolved Hide resolved
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True
)
Copy link
Owner

Choose a reason for hiding this comment

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

Popen cannot be used in Moonraker's code as calls to communicate() block the asyncio event loop. Moonraker has a shell_command component that wraps an async implementation of subprocess that can be used for this purpose:

shell_cmd: SCMDComp = self.server.lookup_component("shell_command")
try:
    response = await shell_cmd.exec_cmd("uhubctrl",  timeout=5.)
except shell_cmd.error as e:
    if e.stderr:
        logging.exception(f"uhubctl returned error: {err.decode(errors='ignore')}")
    raise

return []

async def init_state(self) -> None:
await self.set_power("on" if self.initial_state else "off")
Copy link
Owner

Choose a reason for hiding this comment

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

Is it wise to have an initial state for this? The GPIO implementation is unique in that GPIOs are exclusively controlled by Moonraker, when the service is restarted their state always gets reset, thus an initial state is always required.

The ubhubctrl state exists outside of Moonraker's control. It may not be desirable to force a particular state each time Moonraker starts.

Copy link
Contributor Author

@Jckf Jckf Oct 16, 2022

Choose a reason for hiding this comment

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

This was added because I always want my printer to be off unless explicitly commanded to be on, but maybe there are other ways to toggle power devices automatically at startup? Or would you accept the option if it was optional, and state was only changed on init if it was specified in the config?

Copy link
Owner

Choose a reason for hiding this comment

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

I recently made initial_state a global option for power devices (I just haven't updated the docs yet). It functions as you suggest, the initial_state defaults to None if not configured, in which case we do not change the state of the device on init. The change to your code should be relatively simple, remove line 1332 as the base class has already fetched the option, and change init_state() to:

async def init_state(self) -> None:
    if self.initial_state is not None:
        await self.set_power("on" if self.initial_state else "off")


async def set_power(self, state: str) -> None:
await self.refresh_status(self._port_status(state))
return
Copy link
Owner

Choose a reason for hiding this comment

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

set_power and refresh_status should handle exceptions similar to how it the gpio implementation handles them. That way we can catch any error, reset the state to error, then re-raise he exception so the front end gets an error.

@Arksine
Copy link
Owner

Arksine commented Oct 16, 2022

Thanks. I took an initial look and found a few things that should be addressed.

moonraker/components/power.py Outdated Show resolved Hide resolved
moonraker/components/power.py Outdated Show resolved Hide resolved
return []

async def init_state(self) -> None:
await self.set_power("on" if self.initial_state else "off")
Copy link
Contributor Author

@Jckf Jckf Oct 16, 2022

Choose a reason for hiding this comment

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

This was added because I always want my printer to be off unless explicitly commanded to be on, but maybe there are other ways to toggle power devices automatically at startup? Or would you accept the option if it was optional, and state was only changed on init if it was specified in the config?

@Arksine
Copy link
Owner

Arksine commented Sep 6, 2024

The power module now supports devices using uhubctl, so I am closing this. Thanks.

@Arksine Arksine closed this Sep 6, 2024
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