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

ADS-B: Add a MAVLink in only type #24107

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

WickedShell
Copy link
Contributor

@WickedShell WickedShell commented Jun 20, 2023

This does two main things:

  1. Fixes a bug where we spammed "ADSB: Found transceiver on channel" after we lost the transciever health report. Unfortunately this was incredibly confusing because we were generating this on a PingRX, which is fundamentally a Rx only device. (To make it worse we spammed this at update() rate of 10Hz, which is highly alarming on the GCS).
  2. It doesn't make sense to send transciever out control messages to a device that is Rx only. This stops us from trying to get a Rx device to transmit

Along these lines it changes the CubePilot boards to default to Rx in only, which is the hardware they ship with by default. ( @bugobliterator )

I chose to keep the existing ADSB_TYPE 1 as not changing behavior, otherwise I would break anyone who currently had setup uAvionix MAVLink out. The other option I considered was adding an options parameter, which would be reasonable, but the initial discussions with @magicrub revolved around adding a new type, which is what this presents.

This was tested using a PingRx receiver, and some extra debug prints. I don't have anything that actually transmits to test with, but adding prints in the opposite case of the else's left me pretty convinced that this is working as intended if you have an actual transceiver.

@timtuxworth
Copy link
Contributor

Could this be generic to accept any kind of ADS-B in via MavLink?

@WickedShell
Copy link
Contributor Author

Could this be generic to accept any kind of ADS-B in via MavLink?

It already did accept any MAVLink ADS-B in. This change actually makes it easier to not send any control packets out.

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 what you're trying to do but moving the "basic" ADSB-In only option to type=5 rubs me the wrong way. There's a lot of documentation out there that says =1 and those who use ADSB-out, especially on MAVLink, are a waaaay smaller audience size. If anything, I'd say lets just rename the current MAVLink class to MAVLink_out and make it type=5 and type=1 does nothing in update()

libraries/AP_ADSB/AP_ADSB.cpp Outdated Show resolved Hide resolved
libraries/AP_ADSB/AP_ADSB.h Outdated Show resolved Hide resolved
_frontend.out_state.chan = -1;
_frontend.out_state.chan_last_ms = 0; // if the time isn't reset we spam the message
Copy link
Contributor

Choose a reason for hiding this comment

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

this worthy of a stand-alone PR.

uAvionix_UCP = 3,
Sagetech_MXS = 4,
None = 0,
uAvionix_MAVLink_In = 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

while we're at it.. how about we just call this "MAVLink_In"

@@ -38,13 +38,18 @@ bool AP_ADSB_uAvionix_MAVLink::detect()

void AP_ADSB_uAvionix_MAVLink::update()
{
// if we are an in only instance then we are done here, the work below is all related to sending outputs
if (_frontend.get_type(_instance) == AP_ADSB::Type::uAvionix_MAVLink_In) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is better as != uAvionix_MAVLink_InOut

@@ -6,7 +6,7 @@ BATT_VOLT_MULT 12.02
BATT2_AMP_PERVLT 39.877
BATT2_VOLT_MULT 12.02
# setup ADSB
ADSB_TYPE 1
ADSB_TYPE 5
Copy link
Contributor

Choose a reason for hiding this comment

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

these can go back to 1

@WickedShell
Copy link
Contributor Author

Dev call question: Change the current setups to be in only, or do as the PR presents and make them in/out and change the default to be in only on the builds that enable this by default. Or we can do as tom suggests and change everything to be in only and break all exsisting in/out setups.

@Hwurzburg Hwurzburg added the WikiNeeded needs wiki update label Jun 27, 2023
@@ -56,7 +56,7 @@ const AP_Param::GroupInfo AP_ADSB::var_info[] = {
// @Param: TYPE
// @DisplayName: ADSB Type
// @Description: Type of ADS-B hardware for ADSB-in and ADSB-out configuration and operation. If any type is selected then MAVLink based ADSB-in messages will always be enabled
// @Values: 0:Disabled,1:uAvionix-MAVLink,2:Sagetech,3:uAvionix-UCP,4:Sagetech MX Series
// @Values: 0:Disabled,1:uAvionix-MAVLink-InOut,2:Sagetech,3:uAvionix-UCP,4:Sagetech MX Series,5:uAvionix-MAVLink-In
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to change the order of the Type values here if you want the "-In" option to be higher on the drop-down list.

I guess users won't be confused by which way is "In" (or "out")?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "Rx" and "RxTx"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ADS-B world tends to talk about "ADS-B out" as a sentence, so it seemed slightly clearer to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

The industry terminology is "-out" or "-in"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants