au-rmr / aurmr_tahoma

ROS workspace for our UR16e workcell
1 stars 1 forks source link

added new state for user retry, added state inside of state machine #7

Closed collindang88 closed 1 year ago

collindang88 commented 1 year ago

Added new state in state machine to user prompt upon grasp failure. If user selects "y," retries. If user selects "n", continues onto next pick. User prompt has hardcoded timeout of 10 seconds. I have tested this and it works. I installed the package pytimedinput with pip in the conda env because it was unavailable for conda.

tenzinhl commented 1 year ago

Why make input time out?

tenzinhl commented 1 year ago

If we wanted to avoid using a pip package this solution uses stdlib for timing out on input: https://stackoverflow.com/questions/1335507/keyboard-input-with-timeout

(Although this only works on Linux)

tenzinhl commented 1 year ago

Other than the minor comments I had seems reasonable to me. As a longer lookahead: I think it might be good to add doc comments to state machine states describing what the state does.

collindang88 commented 1 year ago

@tenzinhl thanks for the feedback.

Input timeout was suggested by Josh in Slack here so that's why I added it in.

I didn't know about the select module for Python, so I can try it out next time I go to the lab if we think we should avoid pip modules. I think pytimedinput is (slightly) more elegant because it actually blocks all characters besides "y" and "n", and I don't need to hit Enter after the input with the timedKey method, but I have no clue if using pip + conda will lead to problems down the road. I would love to get some insight on whether this will actually lead to problems in our project or not.

By python doc comments you mean Python docstrings, correct? I've never done these before, but I suppose we can add these in if y'all think the state machine names aren't already sufficient.

tenzinhl commented 1 year ago
  1. Sounds good.

  2. Yeah, my main concern was using Pip, but at this point there's so many pip packages maybe it doesn't matter haha.

  3. Yes. I think long-term it makes sense for every state to have a docstring. They don't have to be long for simple states, but as part of a larger effort to make the codebase more learnable it'd be good. I think a good docstring will provide a high level overview of what the side effects of the state are, and what it's return statuses mean.