Skip to content

Conversation

@ipa-vsp
Copy link
Member

@ipa-vsp ipa-vsp commented Nov 3, 2023

This PR:

  • Timeout for booting all the slaves.
  • Get a non-transmit timeout from bus.yml

@ipa-vsp
Copy link
Member Author

ipa-vsp commented Nov 8, 2023

#194

Copy link
Member

@hellantos hellantos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ipa-vsp can you do these changes?

reset_all_nodes; Specifies whether all slaves shall be reset in case of an error event on a mandatory slave (default: false, see bit 4 in object 1F80).
stop_all_nodes; Specifies whether all slaves shall be stopped in case of an error event on a mandatory slave (default: false, see bit 6 in object 1F80).
boot_time; The timeout for booting mandatory slaves in ms (default: 0, see object 1F89).
boot_timeout; The timeout for booting all slaves in ms (default: 100ms).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100ms seems very short fot a default. I would say go with something like 2 seconds.

{
RCLCPP_WARN(
this->node_->get_logger(),
"No timeout parameter found in config file. Using default value of 100ms.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to boot timeout, otherwise it is hard to get.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in: #385

@roncapat
Copy link
Contributor

roncapat commented Jan 4, 2024

Any update on this?

@ipa-vsp ipa-vsp merged commit 9df35ad into ros-industrial:master Mar 11, 2024
@ipa-vsp ipa-vsp deleted the feature/timeout-config branch March 15, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants