Skip to content
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.

Sign up for GitHub

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

Bugfix/blocking uxr create session #351

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Bugfix/blocking uxr create session #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ set(UCLIENT_MAX_INPUT_BEST_EFFORT_STREAMS 1 CACHE STRING "Set the maximum number
set(UCLIENT_MAX_INPUT_RELIABLE_STREAMS 1 CACHE STRING "Set the maximum number of input reliable streams for session.")
set(UCLIENT_MAX_SESSION_CONNECTION_ATTEMPTS 10 CACHE STRING "Set the number of connection attemps.")
set(UCLIENT_MIN_SESSION_CONNECTION_INTERVAL 1000 CACHE STRING "Set the connection interval in milliseconds.")
if(${UCLIENT_MIN_SESSION_CONNECTION_INTERVAL} LESS_EQUAL 0)
message(FATAL_ERROR "UCLIENT_MIN_SESSION_CONNECTION_INTERVAL is out of range")
endif()
set(UCLIENT_MIN_HEARTBEAT_TIME_INTERVAL 100 CACHE STRING "Set the time interval between heartbeats in milliseconds.")
set(UCLIENT_UDP_TRANSPORT_MTU 512 CACHE STRING "Set the UDP transport MTU.")
set(UCLIENT_TCP_TRANSPORT_MTU 512 CACHE STRING "Set the TCP transport MTU.")
Expand Down
3 changes: 3 additions & 0 deletions include/uxr/client/core/session/session.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ UXRDLLAPI void uxr_set_performance_callback(
/**
* @brief Creates a new session with the Agent.
* This function logs in a session, enabling any other XRCE communication with the Agent.
* It blocks for a time proportional to
* (UXR_CONFIG_MAX_SESSION_CONNECTION_ATTEMPTS * UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL)
* until a response from the Agent is received.
* @param session A uxrSesssion structure previously initialized.
* @return true in case of successful session establishment, and false in other case.
*/
Expand Down
17 changes: 13 additions & 4 deletions src/c/core/session/session.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
#include <uxr/client/profile/multithread/multithread.h>
#include "../../profile/shared_memory/shared_memory_internal.h"

#include <assert.h>
#include <limits.h>

#ifdef UCLIENT_PROFILE_SHARED_MEMORY
#define PROFILE_SHARED_MEMORY_ADD_SIZE 21
#else
Expand Down Expand Up @@ -749,13 +752,19 @@ bool wait_session_status(
{
send_message(session, buffer, length);

int64_t start_timestamp = uxr_millis();
int remaining_time = UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL;
const int64_t start_timestamp = uxr_millis();
int64_t remaining_time = UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL;
assert(remaining_time > 0);
Copy link
Member

Choose a reason for hiding this comment

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

We are not using assert in this project's code base, this can lead to portability issues.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I thought this library was C99 complaint.

Are the users with incompatible C support able to set NDEBUG all the time then?

If not, can you give an example of the desired error propagation method?

Copy link
Member

Choose a reason for hiding this comment

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

It is preferred to check for the underflow and return false as far as wait_session_status was not successful


do
{
listen_message(session, remaining_time);
remaining_time = UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL - (int)(uxr_millis() - start_timestamp);
assert(remaining_time <= INT_MAX); // Protect the narrowing conversion
listen_message(session, (int)remaining_time);
const int64_t current_timestamp = uxr_millis();
assert(current_timestamp >= start_timestamp);
const int64_t elapsed_time = current_timestamp - start_timestamp;
assert(UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL >= elapsed_time);
remaining_time = UXR_CONFIG_MIN_SESSION_CONNECTION_INTERVAL - elapsed_time;
} while (remaining_time > 0 && session->info.last_requested_status == UXR_STATUS_NONE);
}

Expand Down