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.

Sign up for GitHub

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

Jump to bottom

AP_Frsky_Telem: added VSpd to telemetry protocol 4 #12352

Merged
merged 2 commits into from
Feb 9, 2020
Merged

AP_Frsky_Telem: added VSpd to telemetry protocol 4 #12352

merged 2 commits into from
Feb 9, 2020

Conversation

yaapu
Copy link
Contributor

@yaapu yaapu commented Sep 19, 2019 •
edited
Loading

The Frsky vario "virtual" sensor was reporting altitude but not vertical speed.
This patch adds VSpd as a new sensor when protocol 4 is selected.

Notes:

  • Just like the OSD code, if for any reason vertical speed is not available from ahrs it falls back to baro climb rate.
  • this also fixes a bug in the sensors looping code

related issue
#12245

@yaapu yaapu added the WIP label Sep 21, 2019
@yaapu yaapu requested a review from tridge September 21, 2019 08:55
@yaapu
Copy link
Contributor Author

yaapu commented Sep 21, 2019

@tridge this is a pretty trivial change but as soon as I enable the WITHSEMAPHORE on ahrs here I get the very same problem we fixed a couple weeks ago by moving the frsky lib to his own thread:
i.e. s.port timing is lost and an LA trace reveals out of sync late packets, by raising priority to TIMER everything works even with the semaphores enabled. This happens only on this very board, on Pixhawk1 this issue is non present. By simply commenting the semaphores all is fine.

@yaapu
Copy link
Contributor Author

yaapu commented Oct 10, 2019

Code has been reorganized:

  • gps and nav structures are updated only when there's no serial input (between polls)
  • gps sensor has been moved to 2byte frsky sensor.
    The old sensor type required 6 packets while the latter only requires 2 yielding a much better response (refresh rate) by the library. The side effect is that the GPS SENSOR NEEDS REDISCOVERY if you were using the older format (is this OK?)

This has been tested on MatekF405-Wing, please check here for further info #12245

@yaapu yaapu added DevCallTopic and removed WIP labels Oct 25, 2019
tridge
tridge previously requested changes Oct 28, 2019
View reviewed changes
libraries/AP_Frsky_Telem/AP_Frsky_Telem.cpp Outdated
@@ -184,7 +185,7 @@ void AP_Frsky_Telem::send_SPort_Passthrough(void)
*/
void AP_Frsky_Telem::send_SPort(void)
{
const AP_AHRS &_ahrs = AP::ahrs();
//const AP_AHRS &_ahrs = AP::ahrs();
Copy link
Contributor

Choose a reason for hiding this comment

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

better to just remove code, not comment out

libraries/AP_Frsky_Telem/AP_Frsky_Telem.cpp Outdated
_SPort.next_sensor_id = SENSOR_ID_FAS;
break;
}
if (++_SPort.vario_call > 2) _SPort.vario_call = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

check coding style (see bit about curly braces on statements}

libraries/AP_Frsky_Telem/AP_Frsky_Telem.cpp Outdated

AP_AHRS &_ahrs = AP::ahrs();

WITH_SEMAPHORE(_ahrs.get_semaphore());
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd prefer to get only one sem at a time (not hold AHRS and baro at same time)

@yaapu
Copy link
Contributor Author

yaapu commented Nov 2, 2019

@tridge I refactored my changes to minimize semaphore's scope

@WillyZehnder WillyZehnder mentioned this pull request Feb 6, 2020
Closed
@yaapu
AP_Frsky_Telem: added VSpd to telemetry protocol 4
111781b 
The Frsky vario "virtual" sensor was reporting altitude but not vertical speed.
This patch adds VSpd as a new sensor when protocol 4 is selected.
GPS frsky sensor is migrated to 2 byte sensor ID and needs rediscovery if using the previous 1 byte version
@tridge
Copy link
Contributor

tridge commented Feb 8, 2020

this fails the plane frsky sport mode test in CI. We either need to change the test, or fix the frsky code if the test is correct

@yaapu
Copy link
Contributor Author

yaapu commented Feb 8, 2020

thanks @tridge,
this patch adds VSPD as a new sensor but also changes the way GPS is sent down the frsky link.
It uses for GPS the newer "2byte" format which sends either latitude or longitude down the frsky link in a single frame vs the older 1byte format which required 3 frames.
The test is expecting the old format and fails.
@peterbarker can you take a look at this please?

@yaapu
autotest:FRSkySPort: added support for VARIO 0x30 and GPS 0x800 senso…
1b9e23a 
…r types

This adds support for the VARIO sensor 0x30 and GPS sensor 0x800 to the autotest suite
@tridge tridge merged commit b0e5bfc into ArduPilot:master Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tridge tridge tridge left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yaapu @tridge @CraigElder