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

Cleanup #21

Merged
merged 4 commits into from
Jul 11, 2019
Merged

Cleanup #21

merged 4 commits into from
Jul 11, 2019

Conversation

samkys
Copy link
Contributor

@samkys samkys commented Jun 18, 2019

This PR is a cleanup of the microstrain_3dm file. It had various tabs and indentation levels making it very hard to read and follow. Also curly brace location was corrected according to:
http://wiki.ros.org/CppStyleGuide section 6.

Tabs were replaced with spaces. Everything compiles and it was tested with the -25 model and it works with that model.

@bsb808
Copy link
Collaborator

bsb808 commented Jun 19, 2019

A quick look at the source looks encouraging. Thank you for taking the time to add the functionality and help with the maintenance!

I'll try to find some time to test the driver with a -45 model later this week.

@samkys
Copy link
Contributor Author

samkys commented Jun 19, 2019

I was trying to separate out the two pull requests, the cleanup and the parameterization but did them in backwards order. I will close #20 since it is included in this PR anyhow.

@samkys
Copy link
Contributor Author

samkys commented Jun 19, 2019

I just came across an interesting case that I cannot test with the -25. Can someone who has a -45 or -35 please test this prior to merging this PR.

  1. Set: publish_imu = false
  2. Set: publish_odom = true

Does your nav message contain orientation information?
I think there might be a setting that we are required to have both either to true or fix some things in the driver. (ie set some AHRS settings are required for the filtered data that is being received from the filtered 0x82 packet data set.)

@tonybaltovski
Copy link
Member

tonybaltovski commented Jun 24, 2019

@samkys would you mind adding roslint test for the formatting? Otherwise, the co-variance change looks good.

@samkys
Copy link
Contributor Author

samkys commented Jun 24, 2019

@tonybaltovski
Do you want me to leave roslint in the CMAKE and in the package.xml?
Do you want me to clean all of the lines of code that roslint says need cleaned up? (Some of the early ones I currently see are things like: #ifndef header guard has wrong style, please use: .... and Should have a space between // and comment

(Just wanted to check prior to spending a lot of time cleaning everything up and find out you had something else in mind.)

@bsb808
Copy link
Collaborator

bsb808 commented Jun 24, 2019

I ran the test @samkys asked for with a -45 on my desk.

  • publish_imu = false
  • publish_filtered_imu = false

The Odometry messages on nav/odom included orientation information, which is what we would want.

Good to go on this end.

@tonybaltovski
Copy link
Member

@samkys that would be ideal to clean-up the package. I can if you just want to add the roslint package.

@samkys
Copy link
Contributor Author

samkys commented Jul 10, 2019

@tonybaltovski I added roslint and ran it. I cleaned up as much as I could but there were a few lines that I'm not sure what to do with. A lot of the errors it was throwing were too long of lines so I wrapped them around. Take a look and see what you think.

@tonybaltovski
Copy link
Member

Awesome, thanks for that!

@tonybaltovski tonybaltovski merged commit db3f119 into ros-drivers:master Jul 11, 2019
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.

3 participants