Skip to content

Fix wait_session_status listen timeout#322

Merged
Acuadros95 merged 3 commits intodevelopfrom
fix/wait_session
Jun 16, 2022
Merged

Fix wait_session_status listen timeout#322
Acuadros95 merged 3 commits intodevelopfrom
fix/wait_session

Conversation

@Acuadros95
Copy link
Copy Markdown
Contributor

@Acuadros95 Acuadros95 commented Jun 16, 2022

Fix wait session timeout and discards any message until create session response.

@Acuadros95 Acuadros95 requested a review from pablogs9 June 16, 2022 09:14
@richiprosima
Copy link
Copy Markdown
Contributor

Build status:

  • Linux Build Status
  • Windows Build Status

@richiprosima
Copy link
Copy Markdown
Contributor

Build status:

  • Linux Build Status
  • Windows Build Status

Signed-off-by: acuadros95 <[email protected]>
@richiprosima
Copy link
Copy Markdown
Contributor

Build status:

  • Linux Build Status
  • Windows Build Status

@Acuadros95 Acuadros95 merged commit 88bb910 into develop Jun 16, 2022
@Acuadros95 Acuadros95 deleted the fix/wait_session branch June 16, 2022 14:52
do
{
listen_message(session, remaining_time);
remaining_time = UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL - (int)(uxr_millis() - start_timestamp);
Copy link
Copy Markdown

@Ryanf55 Ryanf55 Mar 4, 2023

Choose a reason for hiding this comment

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

This causes invalid underflow when the uxr_millis() returns something invalid, such as the embedded device does not have GPS time yet. I could use some info on how this is being used internally. Our embedded device has multiple time sources. The boot time is what we used to fix this bug, however that does not comply with using ROS2 time (UTC). If we switch time sources at runtime, it would be good to know what to do (from boot time to RTC time).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Which implementation of uxr_millis() are you using?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Our software runs on multiple OS's. This one is ChibiOS, so we had to write our own implentation of uxr_millis(). It was not clear from the public API on the requirements for that function. I filed this bug for more info:
#350

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