-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
Add plugin warnings #439
Add plugin warnings #439
Conversation
By oversight we did not have the special warnings enabled for the plugins.
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.
Nice! Thanks a lot!
@@ -330,7 +330,7 @@ void CalibrationImpl::process_statustext(const mavlink_message_t &message) | |||
// FALLTHROUGH | |||
case CalibrationStatustextParser::Status::CANCELLED: | |||
_calibration_callback = nullptr; | |||
_state == State::NONE; | |||
_state = State::NONE; |
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 catch :)
plugins/telemetry/telemetry.cpp
Outdated
std::numeric_limits<float>::epsilon() && | ||
abs(lhs.velocity.north_m_s - rhs.velocity.north_m_s) <= | ||
fabs(lhs.position.east_m - rhs.position.east_m) <= |
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.
Right.
We need to reserve one char for null termination.
We want to try to avoid to use abs/fabs and instead use std::abs/std::fabs.
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.
Small comment, but generally fine for me.
#ifdef WINDOWS | ||
#ifndef _USE_MATH_DEFINES | ||
#define _USE_MATH_DEFINES | ||
// Instead of using the constant from math.h or cmath we define it ourselves. This way |
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.
👍
MAVLinkParameters::ParamValue value; | ||
value.set_uint32(mavlink_camera_mode); | ||
value.set_uint32(uint32_t(mavlink_camera_mode)); |
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.
What about static_cast<uint32_t>(mavlink_camera_mode)
? Looks more modern to me 😅
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.
True.
I'm merging this and ignoring the jenkins CI for the branch. I think it fails because it greps for warning which is the name of the branch and therefore triggers when it should no. |
Add plugin warnings
By oversight we did not have the special warnings enabled for the plugins.
This enables the warnings again and fixes a few that crept in. And one was actually a bug so that confirms that warnings are important!