-
Notifications
You must be signed in to change notification settings - Fork 9
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
BufferManager: add configuration methods and examples enhancement #53
Conversation
README.md
Outdated
{"two",{1,1}} }, 3); | ||
yarp::telemetry::BufferManager<int32_t> bm(n_samples); | ||
bm.setFileName("buffer_manager_test.mat"); | ||
ChannelInfo var_one{ "one", {1,1} }; |
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 yarp::telemetry
is missing in several of this examples.
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.
For now I put yarp::telemetry
in the latest example that is the one that is in the README, we should remove all the using namespace yarp::telemetry
std::map<std::string, dimensions_t> m_dimensions_map; | ||
bool m_auto_save{false}; | ||
size_t m_n_samples{0}; | ||
std::unordered_map<std::string, Buffer<T>> m_buffer_map; |
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 guess that also the corresponding include should change
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.
Thanks for the tip, I didn't know that there was a specific header!
181b322
to
1ed6276
Compare
m_buffer_map.insert(std::pair<std::string, yarp::telemetry::Buffer<T>>(s.m_var_name,Buffer<T>(n_samples))); | ||
m_dimensions_map.insert(std::pair<std::string, yarp::telemetry::dimensions_t>(s.m_var_name, s.m_dimensions)); | ||
} | ||
assert(addChannels(channels) == true); |
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.
With @AlexAntn we spotted that is a bug, in Release
the addChannels
is not invoked because assert
is skipped
This PR adds some configuration methods, now you are no more forced to configure the BufferManager at the construction phase.
The examples have been cleaned up and refactored accordingly.
Please review code.
cc @S-Dafarra