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

Antenna tracker: mend most things #10452

Merged
merged 2 commits into from
Mar 17, 2019
Merged

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Feb 7, 2019

This fixes a few of the things broken with tracker. However still some issues with the pass through telemetry. I have been connecting directly to tracker with USB. When connected to copter it will only get its location back and a few messages, however with plane it seems to get back all the information it should. When connected to plane or copter there is no GCS control of either tracker or the remote vehicle. My theory is that no messages are getting to tracker or passed on, therefore only getting back what information is streamed by default? Before any connection is made to the remote vehicle tracker behaves as it should can change modes and parameters. Would be great full for some advise on the this pass through issue.

Anyway fixed in this PR are:

  • Starting lat and long prams now work as they should, and tracker reports its location as the one it is using to track with
  • Can now arm and disarm from mission planner, I did this by removing this if (packet.target_component == MAV_COMP_ID_SYSTEM_CONTROL) not sure what it does, couldn't find any other uses of it in the code-base
  • Mission library is constructed, this is not used but it fixes annoying failed to update home location warnings in MP, I added placeholder mission commands that all return false.
  • Added gcs pid mask to report back the pid gains for pitch and yaw as with rover. Potentially useful but untested due to being locked out of all control once the pids are used and a remote vehicle has connected.

@@ -278,6 +290,11 @@ class Tracker : public AP_HAL::HAL::Callbacks {
void tracking_manual_control(const mavlink_manual_control_t &msg);
void update_armed_disarmed();

// Mission library
AP_Mission mission{
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the mission object for anything? we don't support missions.. is it required in order to hold the home position?

Copy link
Member Author

Choose a reason for hiding this comment

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

its just so we can use the GCS mission messages, the mavlink gcs common returns errors to mission planner when it tries to read way point zero on arming. The fix is to construct the mission library so its not nulptr, the other way to fix is add custom mavlink messages for all the mission messages that know not to look for the mission library.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worthwhile to change AP_Mission to check for nullptr callbacks?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, sounded like such a good idea that I went ahead and did it. See #10542

Using that, you probably don't need to create a mission instance. if you did, you could just pass it nullptr functions instead of these empty ones.

@rmackay9
Copy link
Contributor

rmackay9 commented Feb 8, 2019

@IamPete1, this looks really good, thanks for taking this on. Just a couple of cosmetic things and then I'll pull it in.

@peterbarker
Copy link
Contributor

Commit messages need fixing.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Two commits need to be folded together.

AntennaTracker/GCS_Mavlink.cpp Outdated Show resolved Hide resolved
AntennaTracker/GCS_Mavlink.cpp Outdated Show resolved Hide resolved
AntennaTracker/GCS_Mavlink.cpp Outdated Show resolved Hide resolved
AntennaTracker/sensors.cpp Outdated Show resolved Hide resolved
AntennaTracker/GCS_Mavlink.cpp Outdated Show resolved Hide resolved
AntennaTracker/GCS_Mavlink.cpp Outdated Show resolved Hide resolved
AntennaTracker/control_auto.cpp Show resolved Hide resolved
AntennaTracker/tracking.cpp Outdated Show resolved Hide resolved
@peterbarker
Copy link
Contributor

peterbarker commented Feb 8, 2019 via email

@IamPete1 IamPete1 changed the title Antenna tracker: mend some things Antenna tracker: mend most things Feb 8, 2019
@IamPete1
Copy link
Member Author

IamPete1 commented Feb 8, 2019

This now also fixes the communications lock out issue, dare i say, maybe we could do a release, I think it would get lots more use with all the new boards supported

@Evan1010
Copy link

Really looking forward to a new release, when possible!

I remember trying and failing to get the 1.0.0 firmware running on anything but a Pixhawk 1 (a PX4-v2.4.8 clone), so supporting more modern hardware (e.g. cheaper boards, better compasses) would be a boon. It was difficult to get an accurate heading in some situations. Now I just need to figure how to tune the PIDs better.

At the very least, having someone take a look at the firmware to fix current bugs is great.

@IamPete1
Copy link
Member Author

@Evan1010 Should be a new beta an a week or two. To help with PID tuning this included gcs_pid mask param that sends back the values to the gcs. If your keen to get started i can build you a firmware.

@magicrub
Copy link
Contributor

let's get this passing CI and re-evaluate for merging!

AntennaTracker/tracking.cpp Outdated Show resolved Hide resolved
@magicrub
Copy link
Contributor

please rebase this, it's failing on the AP_Logger compile failure thing that snuck in briefly. The particular commit you're basing this on is broken. current master is better

@IamPete1
Copy link
Member Author

@magicrub re-based and fixed that if, hopefully should pass checks now.

Thanks for taking the time to have a look at this.

@rmackay9
Copy link
Contributor

rmackay9 commented Feb 19, 2019

action from the dev call is for @peterbarker to have another look now that it's been rebased.

The rebase has sadly been broken again so perhaps @IamPete1 could rebase again.. or maybe Peter can just ignore that rebase issue as part of his review

Copy link
Contributor

@magicrub magicrub left a comment

Choose a reason for hiding this comment

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

I see 1.5 changes needed. The params are still being corrupted and @peterbarker noted something about the pitch2srv angles

AntennaTracker/Parameters.h Outdated Show resolved Hide resolved
AntennaTracker/control_auto.cpp Show resolved Hide resolved
@peterbarker
Copy link
Contributor

Things aren't factored quite right in the commits here.

I've pulled out the compass functions here: #10653

@IamPete1 IamPete1 force-pushed the Antena_tracker branch 3 times, most recently from 8bb56df to f136237 Compare March 2, 2019 12:22
@IamPete1
Copy link
Member Author

IamPete1 commented Mar 2, 2019

@peterbarker once #10664 has gone in this is all i have left in this PR.
Thanks for all your help getting this sorted

@IamPete1
Copy link
Member Author

IamPete1 commented Mar 9, 2019

I think this just needs a merge now, nothing controversial left

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Should be two commits.

Which mission commands currently fail without the instantiation of the mission object?

AntennaTracker/control_auto.cpp Outdated Show resolved Hide resolved
@IamPete1
Copy link
Member Author

@peterbarker Mission planner (possibly also otters) attempts to read waypoint zero on arming resulting in a error. I thought it was easier to construct the mission library than to add a special case to catch that request.

@peterbarker
Copy link
Contributor

peterbarker commented Mar 10, 2019 via email

@IamPete1
Copy link
Member Author

@peterbarker the issue is still there on master, I think mission planner does it for all vehicles

@peterbarker
Copy link
Contributor

Merged, thanks!

@peterbarker peterbarker merged commit 392b59d into ArduPilot:master Mar 17, 2019
@IamPete1 IamPete1 deleted the Antena_tracker branch November 5, 2019 15:39
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