Skip to content

Update memory inits and fixes - #61

Merged
julionce merged 3 commits into
dashingfrom
feature/dual_transports
May 6, 2020
Merged

Update memory inits and fixes#61
julionce merged 3 commits into
dashingfrom
feature/dual_transports

Conversation

@pablogs9

Copy link
Copy Markdown
Member

This PR:

  • Fix init options copy on rmw
  • Fix timeout management on rmw_wait
  • Adds support for multiple thread usage of rmw static memory pools

Comment thread rmw_microxrcedds_c/src/rmw_wait.c Outdated

uxr_run_session_until_timeout(session, timeout);
if(!uxr_run_session_until_timeout(session, timeout)){
ret = RMW_RET_TIMEOUT;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we return at that point?

@pablogs9 pablogs9 Apr 30, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, because the run_session may timeout but there may be some subscription/service ready for a previous call. They are checked in the following lines.

Assume a case where a certain rmw_wait retrieves two subscriptions and they are buffered and ready to rmw_take. On the app loop the user rmw_takes one of them and call rmw_wait again, this second rmw_wait will timeout in its uxr_run_session_until_timeout but there is a subscription buffered ready to take.

Comment thread rmw_microxrcedds_c/src/rmw_wait.c Outdated
if (!custom_service->micro_buffer_in_use){
services->services[i] = NULL;
}else{
ret = RMW_RET_OK;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we return at line 103, we don't need to reset the return value again.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same as above

Comment thread rmw_microxrcedds_c/src/rmw_wait.c Outdated
if (!custom_client->micro_buffer_in_use){
clients->clients[i] = NULL;
}else{
ret = RMW_RET_OK;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment as before.

Comment thread rmw_microxrcedds_c/src/rmw_wait.c Outdated
if (!custom_subscription->micro_buffer_in_use){
subscriptions->subscribers[i] = NULL;
}else{
ret = RMW_RET_OK;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment as before.

@julionce julionce assigned pablogs9 and unassigned julionce Apr 30, 2020
@pablogs9 pablogs9 assigned julionce and pablogs9 and unassigned pablogs9 and julionce Apr 30, 2020
@pablogs9

Copy link
Copy Markdown
Member Author

The new logic is:

run_session_status buffered_status return
false false RMW_RET_TIMEOUT
false true RMW_RET_OK
true false RMW_RET_ERROR if run_session returns true there must be any element buffered compulsory?
true true RMW_RET_OK

@pablogs9 pablogs9 assigned julionce and unassigned pablogs9 Apr 30, 2020
@julionce
julionce merged commit f6d2ef6 into dashing May 6, 2020
@julionce
julionce deleted the feature/dual_transports branch May 6, 2020 14:47
@pablogs9 pablogs9 mentioned this pull request May 19, 2020
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.

2 participants