-
Notifications
You must be signed in to change notification settings - Fork 8
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
[WIP] Feature/multi robot #19
base: main
Are you sure you want to change the base?
Conversation
…v variable within the new session i.e. for docker names
…th special chars.
b92fe86
to
db59c48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should add a solution for https://github.com/4am-robotics/orga/issues/6838 to this PR, too
robmuxinator/robmuxinator.py
Outdated
@@ -472,7 +472,7 @@ def __init__(self, ssh_client, session_name, yaml_session, envs=None) -> None: | |||
command_env_prefix = "" | |||
if self._envs is not None: | |||
for env in self._envs: | |||
command_env_prefix += "export {}={} && ".format(env[0], env[1]) | |||
command_env_prefix += "export {}='{}' && ".format(env[0], env[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this breaks something for our upstart files, i.e.:
PKG_NAV_ENV_CONFIG="'\$(find mojin_default_env_config)'"
will no longer be correctly passed to the tmux session...
@Odin-byte
do you remember the reason why this is important for you and the multi-robot usecase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this to correctly parse longer strings including special characters. If there is another solution to this problem, this is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I might just need to enclose longer strings in double quotes like your example to ensure correct parsing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you give an example for a string that was problematic in you case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it was longer strings like:
XAUTHORITY='/run/user/1000/.mutter-Xwaylandauth.I7PAU2'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I can image the simpler fix would be to add a pair of double quotes like you did above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem was rather the ROSCONSOLE_FORMAT
variable
putting it into export ROSCONSOLE_FORMAT="'[${severity}] [${time}]: ${message}'"
solved the issue for @Odin-byte
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fmessmer it would be good if you could test the correct usage of this variable, because after wrapping it using double and single quotes the error was solved but the logging format was broken. The consoles only showed empty brackets without the severity, time or message.
ref https://github.com/4am-robotics/orga/issues/4389
ref https://github.com/4am-robotics/orga/issues/6785
modified version of #15
provided as new branch in order to not interfere with @Odin-byte thesis