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

Feature/piksi multi cpp #101

Merged
merged 29 commits into from
Oct 8, 2019
Merged

Feature/piksi multi cpp #101

merged 29 commits into from
Oct 8, 2019

Conversation

rikba
Copy link
Collaborator

@rikba rikba commented Oct 7, 2019

A first shot for a cpp ROS driver as a response to the performance issues with the python driver #98 .

As of now this implementation includes

  • A device base class that contains factory methods to create all possible piksi devices (USB, Ethernet, Serial).
  • A USB implementation that automatically detects attached Piksis and open, reads and closes the ports.
  • A ROS interface that owns all devices and creates SBP message callbacks
  • A ROS node managing opening, reading and closing of all devices.

This minimum working implementation is merely complete and a starting point for design discussions and further implementations, such that additional interfaces, message callbacks and UDP broadcasting.

Currently we use <3% of a i7-7820HQ core which is subject to further optimization on how the polling can be done.

michaelpantic
michaelpantic previously approved these changes Oct 8, 2019
Copy link
Member

@michaelpantic michaelpantic left a comment

Choose a reason for hiding this comment

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

Awesome, really nice!

See comment about publisher architecture (this could simplify our life later), but we can also merge it like this and figure this out in the next PR.

Comment on lines +68 to +70
auto pub_vec = static_cast<std::vector<ros::Publisher>*>(context);
if (!pub_vec) return;
pub_vec->front().publish(ros_msg_heartbeat);
Copy link
Member

Choose a reason for hiding this comment

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

This 3 lines wil be written in every callback, so I would factor them out into another static method.
On a second thought, we might need to change this a it, as we probably don't always have the clear assignement one sbp messsage => corresponds to one ros::publisher.

hmm maybe the context pointer here could be the PiksiMulti Instance itself, and then we have some ENUM Map to get the publishers? But I think there could even be a cleaner solution, we can discuss today ;-)

piksi_multi_cpp/src/piksi_multi.cc Outdated Show resolved Hide resolved
piksi_multi_cpp/CMakeLists.txt Show resolved Hide resolved
@michaelpantic
Copy link
Member

Also we can have a discussion on how to make it nicely multithreaded and stuff. But can also happen at a later stage.

@rikba rikba merged commit 522c568 into master Oct 8, 2019
@rikba rikba deleted the feature/piksi_multi_cpp branch October 8, 2019 08:58
@rikba rikba mentioned this pull request Oct 8, 2019
23 tasks
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