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

HWP Encoder Update (Restart process) #738

Merged
merged 5 commits into from
Sep 16, 2024
Merged

Conversation

bbixler500
Copy link
Contributor

Description

Adds a restart task to the agent, similarly to what is done for the hwp-gripper agent in #737. This task, when called, will remotely restart the process running separately on a beaglebone microcontroller.

Motivation and Context

Currently, any time there is a power cycle to the hwp enclosure, a remote user needs to manually ssh into the encoder beaglebone microcontrollers and restart a process on each. This is both a hassle and requires an expert's knowledge. The restart task condenses this process into a single button on OCS-Web which can be run by anyone.

How Has This Been Tested?

This task has been tested to work on the satp2 hwp. The encoder processing portion of the code is completely unaffected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@bbixler500 bbixler500 requested review from ykyohei and jlashner August 27, 2024 20:58
Copy link
Contributor

@ykyohei ykyohei left a comment

Choose a reason for hiding this comment

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

This is great, thank you very much!
I have one request, if there is a setup requirement for restart, I would like to have a brief document for it in the README of sobonelib or other appropriate places. What's the setup method? Do we just need to use latest sobonelib? I assume that the sobonelib have to be placed in specific path or something, is it correct?

@bbixler500
Copy link
Contributor Author

Agreed that I need to add additional information to the README of sobonelib of how these changes effect things. To give a brief summary, the processes have changed from a script which is running in some terminal session into a service located in the /etc/systemd/system/ folder (similar to how the hostmanager functions). These services are setup to restart automatically whenever there is a powercycle/crash and everything required for the setup is included into the Makefile (so it doesn't matter where the sobonelib directory is located). To implement the change all you need to do is run the command make in the hwp_encoder directory. Once that's done, the process will be running in the background without the need for a terminal session. For debugging purposes the old method will still work, so long as you disable the service first.

Copy link
Contributor

@ykyohei ykyohei left a comment

Choose a reason for hiding this comment

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

Thank you for adding the instruction in README of sobonelib.
I also tested this in satp3 and this is working now. Just in case I will make sure that this does not lead to increase of packet drop etc and then approve this PR.

Copy link
Contributor

@ykyohei ykyohei left a comment

Choose a reason for hiding this comment

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

I confirmed there is no packet drops after start using this.

Copy link
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

Just a couple of comments/questions from me.

socs/agents/hwp_encoder/agent.py Outdated Show resolved Hide resolved
socs/agents/hwp_encoder/agent.py Show resolved Hide resolved
@BrianJKoopman BrianJKoopman self-requested a review September 16, 2024 16:07
Copy link
Member

@BrianJKoopman BrianJKoopman left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me!

@jlashner, did you want to take a look? Else we can squash and merge.

Copy link
Collaborator

@jlashner jlashner left a comment

Choose a reason for hiding this comment

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

Looks good to me! Excited to have this functionality.

@BrianJKoopman BrianJKoopman merged commit b2a80ed into main Sep 16, 2024
7 checks passed
@BrianJKoopman BrianJKoopman deleted the hwp-encoder-restart branch September 16, 2024 16:28
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.

4 participants