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.

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

sys_status.cpp: fix enabling of mem_diag and hwst_diag #1712

Merged
merged 2 commits into from
Feb 21, 2022

Conversation

amilcarlucas
Copy link
Contributor

No description provided.

@amilcarlucas
Copy link
Contributor Author

This makes the diagnostics run, tested in HW

Copy link
Member

@vooon vooon left a comment

Choose a reason for hiding this comment

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

That potentially can have threading access trouble because last_rcd isn't atomic (and can't be), so we have to use either mutex or std::atomic<uint64_t> to store stamp nanoseconds.

struct A {
  std::atomic<uint64_t> stamp;
};

a.stamp = ros::Time::now().toNSec();

auto stamp = ros::Time::fromNSec(a.stamp.load());

But overall looks ok.

@vooon vooon added this to the Version 1.14 milestone Feb 18, 2022
…publish on the diagnostics

Use atomic variable to prevent potential threading problems
@amilcarlucas
Copy link
Contributor Author

fixed, good catch

Copy link
Member

@vooon vooon left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@vooon vooon merged commit 2c2be88 into mavlink:master Feb 21, 2022
@amilcarlucas amilcarlucas deleted the fix-disabled-diagnostics branch February 21, 2022 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants