-
-
Notifications
You must be signed in to change notification settings - Fork 529
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 Status Text #733
Add Status Text #733
Conversation
|
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.
Looking pretty good to me! The notes are probably more for Julian, I'm waiting to learn from his answers 👀.
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.
A couple of comments but overall looks good. Thanks a lot!
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.
Something with the build fails now, can you check?
@julianoes I just tested locally and it builds fine. Just updated everything from develop, |
03e1a09
to
7c09c14
Compare
@douglaswsilva ok I've squashed your commits and removed the unrelated style changes (I have the same problem on Arch Linux though). Let's see if CI is happy. |
@douglaswsilva ok it doesn't build yet if you include the backend:
|
This includes the addition for StatusText.
Presumably the proto submodule was not updated. I've checked out master of the submodule and commited that. |
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.
Missing some doc, and then I believe we're good to go!
Generating docs for nested compound dronecode_sdk::Offboard::Attit/tmp/jenkins/workspace/Dronecode_DronecodeSDK_PR-733/install/include/dronecode_sdk/plugins/telemetry/telemetry.h:147: warning: Member type (variable) of class dronecode_sdk::Telemetry::StatusText is not documented.
…ics/DronecodeSDK into douglas/mavlinkDebugLogs # Conflicts: # plugins/telemetry/include/plugins/telemetry/telemetry.h
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.
LGTM, thanks a lot @douglaswsilva!
I fixed the style, updated the proto submodule and merged develop
. It passes all unit tests (backend + core) on my end.
Oh, still missing documentation:
|
This includes the addition for StatusText.
612bdb6
to
f8d20ca
Compare
…ics/DronecodeSDK into douglas/mavlinkDebugLogs # Conflicts: # plugins/telemetry/include/plugins/telemetry/telemetry.h
@JonasVautherin I added the description (actually it seems like one of you guys had done it already), but let me know if still doesn't work. |
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 good to me! Thanks @douglaswsilva!
Get status text through the SDK.