-
Notifications
You must be signed in to change notification settings - Fork 43
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
Thrusters, Logging, Cleanup, Tests #218
Conversation
DSsoto
commented
Apr 13, 2017
- Fixed flipped node id for nodes 0 and 1 in thruster mapper launch.
- Added throttled logging for dvl (replaces Reduce frequency of dvl error spews on loss of beam #215 )
- Fixed bug with path_marker unit tests due to chaged file name
- Code cleanup for better juju
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.
Good juju, going to increase sanity factor 1.2x. Must be merged with uf-mil/mil_common#33
size_t written = 0; | ||
while (written < str.size()) { | ||
while (written < str.size()) | ||
{ |
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.
Are you using any sort of auto formating for this? We should write a script / add a wiki page for auto formating. Here's a dudes clang formatter config for ROS Style Guide
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.
That wouln't be a bad idea. I didn't use one.
# ${OpenCV_INCLUDE_DIRS} | ||
# bfd | ||
# dl | ||
# ) |
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.
We should start enforcing no dead code unless there's a good reason
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 agree
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.
Is there a reason here?
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.
In most cases, there is little chance it will ever be uncommented so it just adds lines to files and makes them harder / more confusing to read. Plus, we have version control if we need to look at an older version of a file.
When #219 was merged, this pull request was rebased onto master and automatically built by the CI server. There were errors that I suggest you check out over on that site. This is likely because this requires your fork of mil_common, so trigger a rebuild once uf-mil/mil_common#33 builds and is merged. |
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.
Have you tested the changes to the dvl driver?
One more thing, change the blueview lauch file to launch from mil_blueview_driver now that it is changed from uf-mil/mil_common#33 |
@mattlangford, yeah I did. |