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

Add extended sys state for Copter #8265

Closed
wants to merge 4 commits into from
Closed

Add extended sys state for Copter #8265

wants to merge 4 commits into from

Conversation

khancyr
Copy link
Contributor

@khancyr khancyr commented Apr 30, 2018

This PR add a new flag to report if we are in landing state, update the takeoff state in auto and guided mode, report land and takeoff state in the extended sys state message. Those are need for better CC control sequences !

@rmackay9
Copy link
Contributor

rmackay9 commented May 3, 2018

This seems like a good idea in general. I've heard a few times from GCS/companion-computer users that they had trouble knowing if the vehicle was landed or not.

@peterbarker
Copy link
Contributor

Coincidentally I also implemented this on the same day that @khancyr did!

For comparison, here are the commits:
peterbarker@0c205a0

(just this specific chunk:)
peterbarker@e5de6ce#diff-4f15a04337259328467a92d8e35bab2fR2756

Those two functions (vtol_state() and landed_state()) are virtual-void.

@khancyr khancyr changed the title Add extended sys state from Copter Add extended sys state for Copter May 14, 2018
@khancyr
Copy link
Contributor Author

khancyr commented May 14, 2018

rebased on master, vtol_state() (default to undefined), remove pure virtual function so all interface don't need to override it by default.

@khancyr
Copy link
Contributor Author

khancyr commented May 17, 2018

rebase and update after mistake on takeoff flag (thanks @Kiwa21)

Copy link
Member

@OXINARF OXINARF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: if you change to Land mode and then get out of it before landing, you'll still be in landing state.

Also, I have a fundamental issue with the approach taken, it's going back to variables spread out in the code base. In my opinion the Mode class should have one or two methods to get the state, that modes can then override.

@@ -50,4 +50,5 @@ class GCS_MAVLINK_Plane : public GCS_MAVLINK
MAV_STATE system_status() const override;

uint8_t radio_in_rssi() const;
MAV_LANDED_STATE landed_state() const override { return MAV_LANDED_STATE_UNDEFINED;};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this overridden if it does the same as the base method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oups forget to remove those for now !

@@ -43,4 +43,5 @@ class GCS_MAVLINK_Sub : public GCS_MAVLINK {
MAV_MODE base_mode() const override;
uint32_t custom_mode() const override;
MAV_STATE system_status() const override;
MAV_LANDED_STATE landed_state() const override { return MAV_LANDED_STATE_UNDEFINED;};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

@khancyr
Copy link
Contributor Author

khancyr commented May 24, 2018

I agree with you. But I will work on that

@WickedShell
Copy link
Contributor

@khancyr Can this be closed in favor of #8587 (and if it's urgently needed then maybe we should just add ext-sys-status to a stream and put #8587 in now)

@khancyr
Copy link
Contributor Author

khancyr commented Jun 25, 2018

Closing for #8587

@khancyr khancyr closed this Jun 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants