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

ADSB: create ADS-B backend split and add Sagetech driver #15576

Closed
wants to merge 5 commits into from

Conversation

magicrub
Copy link
Contributor

This adds a driver for the Sagetech ADSB transceiver. The ADSB wiki page is getting a little out of date so I plan to update that as well.

  • Created ADSB_Backend
  • abstracted MAVink specific handling into it's own AP_ADSB_MAVLink driver for existing MAVLink hardware (uAvionix)
  • Added Sagetech driver for the XP protocol. There is an MX protocol for newer products that is also available but not yet implemented.
  • adds math library of byte parsing to load arrays
  • adds math library to convert math bases (example: dec 12345 to 0x12345)

This backend split is needed because there is already a Sagetech MX protocol and uAvionix GDL90/UCP protocol in the pipeline.

This is a re-do of PR #15123

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.

More patches required. Too lumpy.

One patch for each logical change.

A patch to create the frontend/backend split, then another that only moves the code between files.

I stopped checking through the mavlink backend part-way through as any issues in there are probably pre-existing. However, since the patch history in this PR doesn't make it clear that no code has changed I'd want to go through it with a fine-toothed comb next review.

github interface was stuffing up association of comments with lines - so things are a little wonky.

@@ -128,6 +128,23 @@ float throttle_curve(float thr_mid, float alpha, float thr_in)
return thr_out;
}

/*
* Convert any base number to any base number. Example octal(8) to decimal(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

So... that return type looks like it might not handle hexadecimal too well.

This function would need a big warning that it's expensive.

}
}

uint16_t fetchU16(const uint8_t *v, bool MSBfirst)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will remove these and create UINT24_VALUE()

libraries/AP_ADSB/AP_ADSB.cpp Show resolved Hide resolved
libraries/AP_ADSB/AP_ADSB.cpp Show resolved Hide resolved
libraries/AP_ADSB/AP_ADSB.cpp Show resolved Hide resolved

// longitude and latitude
// NOTE: these MUST be done in double or else we get roundoff in the maths
const double lon_deg = longitude * (double)1.0e-7 * (longitude < 0 ? -1 : 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no just use fabs?

Comment on lines +688 to +689
const Vector2f speed = AP::ahrs().groundspeed_vector();
float speed_knots = norm(speed.x, speed.y) * M_PER_SEC_TO_KNOTS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const Vector2f speed = AP::ahrs().groundspeed_vector();
float speed_knots = norm(speed.x, speed.y) * M_PER_SEC_TO_KNOTS;
const Vector2f speed = AP::ahrs().groundspeed_vector();
float speed_knots = speed.length() * M_PER_SEC_TO_KNOTS;

// time
uint64_t time_usec;
if (!AP::rtc().get_utc_usec(time_usec)) {
memset(&pkt.payload[36],' ', 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

sizeof

}
}

const char* AP_ADSB_Sagetech::systemStatsBits_to_str(const SystemStateBits systemStateBits)
Copy link
Contributor

Choose a reason for hiding this comment

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

This really doesn't do what its name hints it does - this is a bitmask and this won't return you anything useful if two bits are set.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should also be static

send_msg(pkt);
}

const char* AP_ADSB_Sagetech::type_to_str(const uint8_t type)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be static

@magicrub
Copy link
Contributor Author

I'll close this PR in favor of #15610 which is a pre-backend split PR. This PR has lots of good comments from @peterbarker which I incorporated into it

@magicrub magicrub closed this Oct 20, 2020
@magicrub magicrub deleted the pr/adsb_backendSplit2 branch November 5, 2020 21:41
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants