-
Notifications
You must be signed in to change notification settings - Fork 16.8k
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
Copter: support MAV_FRAME_LOCAL_NED in MISSION_ITEM for auto mode #8748 (rebased) #15854
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.
remove the temp.cpp file
My bad, removed it. |
I'm a little bit confused about why the test failed, I don't understand what does the DynamicNotch mean, per the test result as following:
@amilcarlucas Can you give some direction? |
a29e361
to
49c4e8a
Compare
And now it fails on cloning MAVProxy....
|
49c4e8a
to
e2aea23
Compare
@amilcarlucas finally it passed all the checks lolllllll |
Nice. yes, sometimes the CI is non-deterministic. |
@amilcarlucas I've tested this feature with a copter for many times. It is really very helpful for my current project on hand, and I believe it would also be needed for many others studying indoor flight. To support those who might need this, I listed some information as following: Here's the video |
ArduCopter/mode_auto.cpp
Outdated
// failure to set destination can only be because of missing terrain data | ||
copter.failsafe_terrain_on_event(); | ||
return; | ||
// set waypoint controller target |
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.
The indentation looks messed up
packet.x = cmd.content.location.lat; | ||
packet.y = cmd.content.location.lng; | ||
|
||
packet.z = cmd.content.location.alt / 100.0f; // cmd alt in cm to 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.
Please use a multiplication instead of a division here.
2a298e7
to
27d54ed
Compare
@amilcarlucas Thanks for your quick response. I got some errors consistently while running tests multiple times (listed below). I dig into the code of test cases, looks like the test failed because of the difference in altitude of mission result. The code states that
|
Can you change back to division and re-test? |
27d54ed
to
8eabaf3
Compare
I did the fixes and created a new PR #16259 |
#16259 is cleaner (no temp file), rebased, squashed and passes all CI tests |
@amilcarlucas ok thanks for your work |
This is based on the work of @chobitsfan with branch and issue #8748.
I've done the rebase on master branch and need someone to check the logic is correct.
@amilcarlucas may I kindly request for your review?