fixing path in granite #149
Conversation
| exit_code = first_non_zero(exit_code, cexec.change_exposure(exposure_value)) | ||
| exit_code = first_non_zero( | ||
| exit_code, self.command_executor.change_exposure(exposure_value) | ||
| ) |
There was a problem hiding this comment.
I had intentionally made this change to create a new CommandExecutor every time because I ran into trouble with another action that reused a CommandExecutor and I thought the problem might have to do with the CommandExecutor having been left in an inconsistent state somehow such that it failed on the second pass.
The logic of CommandExecutor state is hard enough to follow that I felt more confident discarding the old one and creating a new one every time. Is there a significant resource concern with doing it that way? (I think it's generally a good idea in terms of isolation.)
I also want to point out that this code is probably not getting tested unless you are really going out of your way to do that (running multi-module scenarios), so we don't really know if either the old or the new version is working.
There was a problem hiding this comment.
Interesting. If there is a bug regarding that, we should focus on fixing the bug instead.
Creating a CommandExecutor instance takes something like 3 seconds because it has to publish and wait for subscribers. It seems like bad practice introducing a lag this big 3 times in a row xD
| @@ -120,15 +120,15 @@ def get_ops_plan_path(): | |||
| # Check if the environment variable $ASTROBEE_OPS exists | |||
| astrobee_ops_path = os.getenv("ASTROBEE_OPS") | |||
| if astrobee_ops_path: | |||
| return os.path.join(astrobee_ops_path, "gds/plans") | |||
| return os.path.join(astrobee_ops_path, "dock_scripts/hsc/gds/plans") | |||
There was a problem hiding this comment.
Obviously it's critical for this to work on the real robot, but I hope it can work in multi-robot software sim as well. Did you check it to see if the instructions here still work?
This looks like it may be depending on an environment variable $ASTROBEE_OPS. If so, the sim instructions should probably be updated to explain how to set that. (Note that the $OPS_REPO variable name wasn't something I made up but rather comes from older wiki instructions.
Note: I'm not saying this is broken, I just haven't had a chance to test it yet and want to make sure the issue is on your radar. Thanks!
There was a problem hiding this comment.
Yeah it still works because there are symlinks inside the ops repo. So doing gds/plans is the same as dock_scripts/hsc/gds/plans, but to the robot, we only copy the folder dock_scripts/hsc/
There was a problem hiding this comment.
regarding the env variable name, that's only used if you want to overwrite the usual path. If you have GDS installed, that's used if no OPS_REPO defined
astrobee/survey_manager/survey_planner/src/survey_planner/problem_generator.py
Outdated
Show resolved
Hide resolved
These paths were not compatible with the real robots, fixing it