Hwp site changes #541

bbixler500 · 2023-10-10T13:39:39Z

Changes to the hwp_gripper, hwp_pid, and hwp_pmx agents to improve functionality at the site

Description

hwp_gripper: fixed how arguments are passed to the agent and renamed a function for consistency
hwp_pid: changed the socket communication to prevent agent crashes
hwp_pmx: changed the socket communication to prevent agent crashes

Motivation and Context

As it is currently, the hwp_gripper agent does not work. The changes made to that agent were made to get it running. The hwp_pid and hwp_pmx agents crash if the network communication is temporarily lost. The changes made to those agents are to prevent that crashing.

How Has This Been Tested?

All three of these agents have been running on the dev environment overnight with no issue. The changes to the hwp_gripper agent are minor and only effect how the agent is initialized. The changes to the hwp_pid and hwp_pmx agents only change the socket communication between agent and device, and the agents have been continually sending data to grafana successfully.

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.

updates: - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

for more information, see https://pre-commit.ci

jlashner

This looks ok to me for the most part, however I don't agree with the new _establish_connection logic in the PID and PMX controllers. I think this needs to fail after a certain number of attempts, instead of entering an infinite loop.

jlashner · 2023-11-01T17:17:34Z

socs/agents/hwp_pid/drivers/pid_controller.py

        # unit tests might fail on first connection attempt
-        attempts = 3
-        for attempt in range(attempts):
+        while True:


I don't love this being an infinite loop, since control is never passed back to the agent. Can we keep this to a finite number of attempts, and handle re-connection at the agent level?

jlashner · 2023-11-01T17:21:26Z

socs/agents/hwp_pmx/drivers/PMX_ethernet.py

+                if attempt == 1:
+                    print("Resetting connection")
+                    self.conn.close()
+                    self.conn = self._establish_connection(self.ip, int(self.port))


as mentioned above, if there is no connection, establish_connection here will wait forever. Instead, this should throw an error after a finite amount of time.

davidvng · 2023-11-17T19:25:24Z

Hi @bbixler500 , this fix looks great! Over at SATP3 we've been having trouble with our PID and gripper agents, and I've tested this branch and they work. I'd greatly appreciate getting this merged soon.

bbixler500 · 2023-11-17T20:23:27Z

At the site there was a strange behavior where the PID and PMX agents would consistently crash every night, requiring me to restart the agents every morning. These crashes occurred because of connection interruptions, which could last up to 3hrs (I was never able to determine the cause of the connection interruptions). Previously the agents were coded so that _establish_connection would throw an error after a set number of unsuccessful connections. Consequently, the number of unsuccessful attempts was always reached and the agent would always crash. The switch to the infinite loop solved the agents crashing issue.

davidvng · 2023-11-17T20:43:33Z

At the site there was a strange behavior where the PID and PMX agents would consistently crash every night, requiring me to restart the agents every morning. These crashes occurred because of connection interruptions, which could last up to 3hrs (I was never able to determine the cause of the connection interruptions). Previously the agents were coded so that _establish_connection would throw an error after a set number of unsuccessful connections. Consequently, the number of unsuccessful attempts was always reached and the agent would always crash. The switch to the infinite loop solved the agents crashing issue.

I think you can let the acq process be the "infinite loop" here: https://github.com/simonsobs/socs/blob/hwp_site_changes/socs/agents/hwp_pid/agent.py#L286
Then, whenever these calls fail, you can call init_connection again. I think this is what @jlashner meant by reconnecting at the agent level.

…f iterations. Added reconnection functionality into the agent acq process

for more information, see https://pre-commit.ci

davidvng · 2023-11-19T19:33:04Z

@bbixler500 this change looks good to me. If you could fix the pytest failure and pre-commit failure, we can get this approved and merged.

bbixler500 · 2023-11-20T23:48:29Z

I'm not sure what this pytest error means or how to solve it. It appears to be from the test_hwp_pmx_agent_integration.py file

socs/agents/hwp_pmx/drivers/PMX_ethernet.py

davidvng · 2023-11-22T16:51:11Z

@jlashner @BrianJKoopman can we get this reviewed and merged this week? SATP3 would like to spin the HWP this weekend. thanks!

jlashner

Looks good to me! Thanks for the changes @bbixler500

* minor bugfixes to the hwp-gripper agent * Modified hwp-pid and hwp-pmx agents to handle connection interruptions * [pre-commit.ci] pre-commit autoupdate (#540) updates: - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Changed connection protocal from an infinite loop into a set number of iterations. Added reconnection functionality into the agent acq process * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fixes to pre-commit failures * Added read=False to set_current_limit meathod --------- Co-authored-by: Bryce Bixler <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Bryce Bixler and others added 4 commits October 9, 2023 18:43

minor bugfixes to the hwp-gripper agent

959fd9b

Modified hwp-pid and hwp-pmx agents to handle connection interruptions

e8623ac

[pre-commit.ci] pre-commit autoupdate (#540)

a50ca05

updates: - [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

fa26efa

for more information, see https://pre-commit.ci

BrianJKoopman requested review from BrianJKoopman and jlashner October 11, 2023 18:58

jlashner requested changes Nov 1, 2023

View reviewed changes

Bryce Bixler and others added 2 commits November 17, 2023 22:26

Changed connection protocal from an infinite loop into a set number o…

f939874

…f iterations. Added reconnection functionality into the agent acq process

[pre-commit.ci] auto fixes from pre-commit.com hooks

f7cc7e4

for more information, see https://pre-commit.ci

Fixes to pre-commit failures

0735a59

davidvng reviewed Nov 21, 2023

View reviewed changes

socs/agents/hwp_pmx/drivers/PMX_ethernet.py Outdated Show resolved Hide resolved

Added read=False to set_current_limit meathod

a7f009f

davidvng requested a review from jlashner November 21, 2023 22:51

jlashner approved these changes Nov 22, 2023

View reviewed changes

BrianJKoopman merged commit fc79849 into main Nov 22, 2023
7 checks passed

BrianJKoopman deleted the hwp_site_changes branch November 22, 2023 23:04

davidvng linked an issue Nov 28, 2023 that may be closed by this pull request

HWP Gripper agent unexpected keyword argument 'mcu_ip' #567

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hwp site changes #541

Hwp site changes #541

bbixler500 commented Oct 10, 2023

jlashner left a comment •

edited

Loading

jlashner Nov 1, 2023

jlashner Nov 1, 2023

davidvng commented Nov 17, 2023

bbixler500 commented Nov 17, 2023

davidvng commented Nov 17, 2023

davidvng commented Nov 19, 2023

bbixler500 commented Nov 20, 2023

davidvng commented Nov 22, 2023

jlashner left a comment

Hwp site changes #541

Hwp site changes #541

Conversation

bbixler500 commented Oct 10, 2023

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

jlashner left a comment • edited Loading

Choose a reason for hiding this comment

jlashner Nov 1, 2023

Choose a reason for hiding this comment

jlashner Nov 1, 2023

Choose a reason for hiding this comment

davidvng commented Nov 17, 2023

bbixler500 commented Nov 17, 2023

davidvng commented Nov 17, 2023

davidvng commented Nov 19, 2023

bbixler500 commented Nov 20, 2023

davidvng commented Nov 22, 2023

jlashner left a comment

Choose a reason for hiding this comment

jlashner left a comment •

edited

Loading