Rover land detection #13769
Rover land detection #13769
Conversation
Merging latest master
| private: | ||
| // Program crashes when Subscriptor declared here | ||
| //uORB::Subscription _vehicle_status_sub{ORB_ID(vehicle_status)}; |
There was a problem hiding this comment.
Can you revert this change and we'll work through whatever issue you hit?
There was a problem hiding this comment.
Hello dagar, apologies for delay, was away for Xmas.
Have just reverted the changes on my branch.
Please let me know if there is anything else I can help with
| uORB::Subscription vehicle_status_sub{ORB_ID(vehicle_status)}; | ||
| vehicle_status_s vehicle_status{}; /**< vehicle status */ | ||
|
|
||
| vehicle_status_sub.update(&vehicle_status); |
There was a problem hiding this comment.
| vehicle_status_sub.update(&vehicle_status); | |
| _vehicle_status_sub.update(&_vehicle_status); |
|
|
||
| // I need to do the uORB subscription here, can't do it in the .hpp file (SIGSEGV if I do... but no idea why) | ||
| uORB::Subscription vehicle_status_sub{ORB_ID(vehicle_status)}; | ||
| vehicle_status_s vehicle_status{}; /**< vehicle status */ |
There was a problem hiding this comment.
| vehicle_status_s vehicle_status{}; /**< vehicle status */ |
| return !_actuator_armed.armed; | ||
|
|
||
| // I need to do the uORB subscription here, can't do it in the .hpp file (SIGSEGV if I do... but no idea why) | ||
| uORB::Subscription vehicle_status_sub{ORB_ID(vehicle_status)}; |
There was a problem hiding this comment.
| uORB::Subscription vehicle_status_sub{ORB_ID(vehicle_status)}; |
| } | ||
| else | ||
| { | ||
| return !_actuator_armed.armed; // If we are armed we are not landed |
There was a problem hiding this comment.
The system being disarmed should probably be checked first to override.
There was a problem hiding this comment.
Hello dagar, I don't really follow what you mean. This part of the code is unchanged.
There was a problem hiding this comment.
I mean you can check _actuator_armed.armed before updating vehicle_status and checking the nav_state.
There was a problem hiding this comment.
Ah, I think my comment in the code might be confusing things. The system works perfectly as it is currently.
The original part of the code (which is now part of the else) is what didn't work properly
else {
return !_actuator_armed.armed; // If we are armed we are not landed
// This doesn't work very well because once it's armed it will stay armed
// so when requesting land it never detects it has landed.
I'll remove those comments, as they are confusing
In the header it caused SIGSEGV in my machine so that's why it was moved to .cpp
|
CI is failing due to code style. http://ci.px4.io:8080/blue/organizations/jenkins/PX4%2FFirmware/detail/PR-13769/2/pipeline |
|
@ealdaz-seesai I'm curious. How does your rover "land" 😄 ? Or what is the use-case to use land with a rover? |
|
@ItsTimmy would be great if you could review this one, thanks! |
|
Ha,ha,ha ... yes, surprising concept for sure. In fact we use land as a proxy to end the mission in the same way as for a drone. We use both drones and rovers so it made sense to us to use the same commands for both.
Open to better ideas or suggestions
Thanks for following up on this pull request as I was about to reach out and enquire if there was anything else I needed to do.
Eduardo
…Sent from my iPhone
On 14 Feb 2020, at 16:05, Julian Oes ***@***.***> wrote:
@ealdaz-seesai I'm curious. How does your rover "land" 😄 ? Or what is the use-case to use land with a rover?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
There was a problem hiding this comment.
I think this is a good solution. The rover needs some way to indicate that it is done with a mission, just like a drone does, so it makes sense to reuse the same concept of "landing", even though the drone isn't really landing.
One thing that could be improved is to check whether the speed of the rover is 0 (or close to 0) before "landing", but I think this would fall under a separate PR. So this one is good as is.
ealdaz-seesai
dismissed stale reviews from julianoes and AmeliaEScott
via
1495fd4
|
Is there anything else required to merge this? |
|
Thanks @ealdaz-seesai! |
|
A pleasure, thank you! |
Describe problem solved by this pull request
In the current implementation when Land is requested the rover never lands but circles continuosly around the position where land was requested.
This is due to the fact that the RoveLandDetector currently returns the arm status, and once the Rover is armed it remains armed and hence land is never achieved.
Describe your solution
In the proposed implementation if landing is requested the Land detector detects that
vehicle_status.nav_state == vehicle_status_s::NAVIGATION_STATE_AUTO_LANDand then simply returns true.
Test data / coverage
This was tested with both px4_sitl gazebo_rover and a pixhawk4 rover running the Aion RObotics R1 UGV (Tank type/Differential drive)
Current Behaviour
Rover circles continously around landing position and landing is never detected
Proposed Behaviour.
Rover stops and landing is detected.