Respawn all option #8
Conversation
| @@ -276,7 +281,7 @@ void LaunchConfig::parseNode(TiXmlElement* element, ParseContext ctx) | |||
| const char* type = element->Attribute("type"); | |||
| const char* args = element->Attribute("args"); | |||
| const char* ns = element->Attribute("ns"); | |||
| const char* respawn = element->Attribute("respawn"); | |||
| const char* respawn = element->Attribute("respawn"); | |||
There was a problem hiding this comment.
this repo uses tabs instead of spaces, please use tabs for this external repo to preserve formatting
rosmon_core/src/main.cpp
Outdated
| @@ -75,6 +75,9 @@ void usage() | |||
| " Use GROUP as name of the launch group. By default, empty\n" | |||
| " --launch-config=CONFIG\n" | |||
| " Use CONFIG as name of the launch config. By default, empty\n" | |||
| " --respawn-all=BOOL\n" | |||
There was a problem hiding this comment.
can you do this similar to xqms#109,
basically have `--respawn-attr="obey|force_true|force_false"
|
https://bitbucket.org/rapyutians/sootballs/src/devel/tools/control_center/control_center/apis.py#tools/control_center/control_center/apis.py-101 you can pass the respawn flag in sootballs code here. Please pass the respawn-attr as a configuration parameter in https://bitbucket.org/rapyutians/sootballs/src/devel/tools/control_center/control_center/settings.py#tools/control_center/control_center/settings.py-8 and load it in the apis.py file. |
rosmon_core/src/main.cpp
Outdated
| @@ -75,6 +75,9 @@ void usage() | |||
| " Use GROUP as name of the launch group. By default, empty\n" | |||
| " --launch-config=CONFIG\n" | |||
| " Use CONFIG as name of the launch config. By default, empty\n" | |||
| " --respawn-attr=force_true|force_false\n" | |||
There was a problem hiding this comment.
as mentioned before lets keep an option to obey the respawn rules mentioned in launch file which will be default.
rosmon_core/src/main.cpp
Outdated
| @@ -75,6 +75,9 @@ void usage() | |||
| " Use GROUP as name of the launch group. By default, empty\n" | |||
| " --launch-config=CONFIG\n" | |||
| " Use CONFIG as name of the launch config. By default, empty\n" | |||
| " --respawn-attr=force_true|force_false\n" | |||
There was a problem hiding this comment.
please check formatting in the file, replace space indentation with tabs. The simplest way to make sure is to view spaces in IDE (https://stackoverflow.com/questions/30140595/show-whitespace-characters-in-visual-studio-code) and replace spaces with tabs (copy from another part part of file)
rosmon_core/src/main.cpp
Outdated
| @@ -133,6 +136,7 @@ static const struct option OPTIONS[] = { | |||
| {"robot", required_argument, nullptr, 'r'}, | |||
| {"launch-group", required_argument, nullptr, 'g'}, | |||
| {"launch-config", required_argument, nullptr, 'z'}, | |||
| {"respawn-all", required_argument, nullptr, 'R'}, | |||
There was a problem hiding this comment.
does not match the help script from above now
| { | ||
| node->setRespawn(ctx.parseBool(respawn, element->Row())); | ||
| node->setRespawn(m_respawnAll || ctx.parseBool(respawn, element->Row())); |
There was a problem hiding this comment.
force_false should override respawn to false. I suggest keeping two parameters m_respawnObey and m_respawnAll.
if (m_respawnObey) {
node->setRespawn(ctx.parseBool(respawn, element->Row()));
} else {
node->setRespawn(m_respawnAll);
}
|
Sorry I still need to cleanup the indenting for this more @gauthamm |
|
Also, check the Travis for changes in the test. |
|
|
||
| if(respawnDelay) | ||
| } | ||
| else |
| node->setRespawn(m_respawnAll); | ||
| } | ||
|
|
||
| if(((m_respawnObey && respawn && ctx.parseBool(respawn, element->Row())) || m_respawnAll) && respawnDelay) |
There was a problem hiding this comment.
I think instead of complicated chain of conditions here, the only change needed in this function would be to swap the following in original implementation
if (respawn)
{
node->setRespawn(ctx.parseBool(respawn, element->Row()));
with
if ((m_respawnObey && respawn) || !m_respawnObey)
{
if (!m_respawnObey) {
node->setRespawn(m_respawnAll);
} else {
node->setRespawn(ctx.parseBool(respawn, element->Row()));
}
There was a problem hiding this comment.
I don't think this would work because then respawn delay will be set even if m_respawnAll is set to false.
| else | ||
| { | ||
| node->setRespawn(ctx.parseBool(respawn, element->Row())); | ||
| } | ||
| if(respawnDelay) |
There was a problem hiding this comment.
I assume you verified this is not an issue?
There was a problem hiding this comment.
Yes, respawndelay is only used if respawn is set.
https://www.wrike.com/open.htm?id=450858821
Option is set with command line flags. If respawn all is set to false but the node's respawn is set to true in launch file, will still set respawn to true. I can change that though.
Also @gauthamm I'm unsure how to implement this with sootballs? Should I do it in apis.py? Or somewhere else?