-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
RSDK-3344 - getPosition refactor #110
RSDK-3344 - getPosition refactor #110
Conversation
…kim-mishra/viam-cartographer into RSDK-3344-refactor-get-position
{ | ||
std::lock_guard<std::mutex> lk(viam_response_mutex); | ||
global_pose = latest_global_pose; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Is there any behavior surrounding lock availability we want to implement here? Or do we always want GetPosition
to wait until the lock is available
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
B/c the viam_response_mutex
is only held for the duration of copying a variable (not an expensive operation) it is ok to block until the lock is acquired.
|
||
auto pos_vector = global_pose.translation(); | ||
auto pos_quat = global_pose.rotation(); | ||
viam_carto_get_position_response* vcgpr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you want to actually instantiate the struct & then pass a reference to it to the function, otherwise I think the pointer could point to uninitialized memory:
viam_carto_get_position_response vcgpr;
GetPosition(&vcgpr);
{ | ||
std::lock_guard<std::mutex> lk(viam_response_mutex); | ||
global_pose = latest_global_pose; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
B/c the viam_response_mutex
is only held for the duration of copying a variable (not an expensive operation) it is ok to block until the lock is acquired.
No description provided.