-
-
Notifications
You must be signed in to change notification settings - Fork 510
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
backend: add health to telemetry #362
Conversation
c7a2a4c
to
5f908cc
Compare
const dronecore::rpc::telemetry::SubscribeHealthRequest * /* request */, | ||
grpc::ServerWriter<rpc::telemetry::HealthResponse> *writer) override | ||
{ | ||
_telemetry.health_async([&writer](dronecore::Telemetry::Health health) { |
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.
Cool
@@ -106,24 +121,46 @@ std::future<void> TelemetryServiceImplTest::subscribePositionAsync(std::vector<P | |||
}); | |||
} | |||
|
|||
TEST_F(TelemetryServiceImplTest, doesNotSendPositionIfCallbackNotCalled) |
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 name: doesNotSendPositionIfCallbackNotCalled
:-)
EXPECT_CALL(*_telemetry, health_async(_)) | ||
.Times(1); | ||
|
||
std::vector<Health> healths; |
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.
Why a vector ? Health
is meant for a system/vehicle isn't it ?
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'm not sure I understand the question. I collect the Health
events sent by the drone in this vector, for the test...
How would you do it?
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.
Looks all correct.
{ | ||
return str << "[gyrometer_calibration_ok: " << health.gyrometer_calibration_ok << | ||
", accelerometer_calibration_ok: " << health.accelerometer_calibration_ok << | ||
", magnetometer_calibration_ok: " << health.magnetometer_calibration_ok << |
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 usually start these lines with <<
and indent it in the same way. Just for next time.
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 did not think about it and let astyle do it. But I prefer you're way. I'll change it.
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.
Well, I'm actually moving that in the next PR, so I'll change it at this time :-).
5f908cc
to
25e0f53
Compare
Fixes #333.