Generic ESC harmonic notch handling #16725

andyp1per · 2021-02-23T22:08:12Z

This is a generic abstraction for ESC telemety which I have ported all of the existing telemetry providers to.
It incorporates dynamic notch support, rpm slew, mavlink and logging support. Overall the code has got smaller.
There is also a new autotest for the dynamic notch support.

amilcarlucas · 2021-02-23T22:50:05Z

Maybe replace HAVE_AP_BLHELI_SUPPORT with HAVE_ESC_TELEM_SUPPORT in some places?

amilcarlucas

Nice stuff, loving it!

libraries/AP_ESC_Telem/AP_ESC_Telem_Backend.cpp

libraries/AP_MSP/AP_MSP_Telem_Backend.cpp

libraries/AP_ToshibaCAN/AP_ToshibaCAN.cpp

libraries/AP_ToshibaCAN/AP_ToshibaCAN.h

libraries/AP_BLHeli/AP_BLHeli.cpp

amilcarlucas · 2021-02-25T13:42:40Z

Implements a part of #16645 together with amilcarlucas#4

libraries/AP_ESC_Telem/AP_ESC_Telem.h

libraries/AP_ESC_Telem/AP_ESC_Telem.cpp

libraries/AP_BLHeli/AP_BLHeli.cpp

libraries/AP_ESC_Telem/AP_ESC_Telem_Backend.h

amilcarlucas

AP_PiccoloCAN and AP_KDECAN support are missing in this PR

libraries/AP_ESC_Telem/AP_ESC_Telem_Backend.cpp

andyp1per · 2021-02-26T22:35:31Z

This is working on my 3" quad with BLHeli bi-dir dshot

andyp1per · 2021-02-27T17:50:00Z

Ok, PiccoloCAN, UAVCAN and KDECAN done. Will need some testing and I am not convinced about the motors API - might need to use masks instead

andyp1per · 2021-02-27T21:13:16Z

Tridge and I have zero'ed in on a slightly different design ...

amilcarlucas · 2021-02-28T23:18:48Z

I think the _telemetry and _telem_semaphore member variables should be moved to the AP_ESC_Telem_Backend base class together with the functions:

    bool get_usage_seconds(uint8_t esc_id, uint32_t& usage_sec) const override;
    bool get_temperature(uint8_t esc_id, int16_t& temp) const override;
    bool get_rpm(uint8_t esc_id, uint16_t& rpm) const override;
    bool get_current_ca(uint8_t esc_id, uint16_t& amps_ca) const override;
    uint32_t have_telem_data(uint8_t esc_id) const override;
    void send_esc_telemetry_mavlink(uint8_t mav_chan) const;
    void log_esc(uint8_t inst);

What do you think?

andyp1per · 2021-03-01T08:18:02Z

The alternate design we came up with was to have an additional callback - update_esc_telemetry() that populated the frontend with the additional telemetry data together with a timestamp. Then callers only need to call the front end - no need for virtual functions at all in the main. This will also cope with multiple drivers populating the ESC data.

libraries/AP_ESC_Telem/AP_ESC_Telem.cpp

libraries/AP_KDECAN/AP_KDECAN.cpp

amilcarlucas

Nice, we are getting there :)

amilcarlucas · 2021-03-03T12:25:25Z

This adds less than 90 lines of code, and reduces FW size by about 1300 bytes (ArduCopter on Durandal) 👍

amilcarlucas · 2021-03-03T12:26:15Z

I do think that it needs a semaphore to ensure the consistency of the telemetry data.

andyp1per · 2021-03-03T12:33:48Z

@amilcarlucas - I've deliberately not done that because I don't want timing critical code (e.g. bi-dir dshot) being required to take out the lock. If we really want a lock then I'll do a backend/frontend copy like we do for the IMUs. But at the moment I don't think its necessary. All the data types are 32bits or less (i.e. atomic) and the only coordinated update is between the timestamp and data type. We only really care about the timestamp for out of data data - at which point nothing is being written anyway so a little out of date is fine. So at the moment I don't think anything bad will happen as its stands and it avoids some contention.
Also indiviudal ESCs have their own individual slot - there is no conflict between them.

remove send_esc_telemetry_mavlink() log telemetry data in frontend record volts, amps and consumption as floats

…ing with other ESCs remove send_esc_telemetry_mavlink() log telemetry data in frontend remove datastructures and semaphore obsoleted by AP_ESC_Telem* (<amilcar.lucas@iav.de>) record volts, amps and consumption as floats

remove send_esc_telemetry_mavlink() remove datastructures and semaphore obsoleted by AP_ESC_Telem* (<amilcar.lucas@iav.de>) record volts, amps and consumption as floats

… with other ESCs log ESC telemetry data in frontend

…h other ESCs

… other ESCs

…ng with other ESCs refactor to capture and output slewed rpm values enable with HAL_WITH_ESC_TELEM move notch calculation to front end refactor telemetry data into frontend cope with blended data add mavlink send function log telemetry data in frontend add SITL ESC telemetry record volts, amps and consumption as floats report telemetry transmission errors disable ESC Telemetry inclusion when there is no need for it move ESC_Telem logging to the AP_ESC_Telem class (by amilcar.lucas@iav.de) various cleanups (by amilcar.lucas@iav.de) add support for raw ESC rpm check RPM validity for mavlink output Use const when applicable

rename AP_BattMonitor_BLHeliESC -> AP_BattMonitor_ESC record volts, amps and consumption as floats Correct ESC-telemetry-based voltage and temperature (<amilcar.lucas@iav.de>) Correct ESC-telemetry-based voltage and temperature when less than 12 ESCs are used (<amilcar.lucas@iav.de>) fix jumps in consumed current (<amilcar.lucas@iav.de>) Implement temperature readings (<amilcar.lucas@iav.de>) Fix temperature scaling (<amilcar.lucas@iav.de>)

andyp1per · 2021-05-07T16:13:44Z

Squashed and rebased

added Ampere hours unit in LOG_ESC_MSG log ESC volts, amps and consumption as floats update ESC log file structures consumption in mAh Correct the current_tot unit, motor_temp unit and error_rate unit in comments (<amilcar.lucas@iav.de>) move ESC_Telem logging to the AP_ESC_Telem class (<amilcar.lucas@iav.de>) correct log structure (<amilcar.lucas@iav.de>)

khancyr

That seems ok. I will do another pass for review later

rmackay9 · 2021-05-10T07:44:18Z

I've re-tested ToshibaCAN ESCs and they also seem to work. So from a basic functionality point of view I think KDECAN and ToshibaCAN telemetry continues to work.

tridge · 2021-05-11T00:49:24Z

i have UAVCAN ESCs to test

tridge · 2021-05-12T06:51:19Z

all working well on a quad with 4 UAVCAN ESCs

fixed

andyp1per changed the title ~~Generic ESC harmoni notch handling~~ Generic ESC harmonic notch handling Feb 23, 2021

andyp1per added Copter Enhancement Plane labels Feb 23, 2021

amilcarlucas mentioned this pull request Feb 24, 2021

AP_FETtecOneWire: implement AP_ESC_Telem_Backend amilcarlucas/ardupilot#4

Open

amilcarlucas suggested changes Feb 24, 2021

View reviewed changes

andyp1per force-pushed the pr-esc-notch branch from 9c448f2 to 0172f74 Compare February 24, 2021 18:32

WickedShell previously requested changes Feb 25, 2021

View reviewed changes

libraries/AP_ESC_Telem/AP_ESC_Telem.h Outdated Show resolved Hide resolved

libraries/AP_ESC_Telem/AP_ESC_Telem.cpp Outdated Show resolved Hide resolved

amilcarlucas suggested changes Feb 26, 2021

View reviewed changes

libraries/AP_ESC_Telem/AP_ESC_Telem_Backend.cpp Outdated Show resolved Hide resolved

libraries/AP_ESC_Telem/AP_ESC_Telem_Backend.cpp Outdated Show resolved Hide resolved

libraries/AP_ESC_Telem/AP_ESC_Telem_Backend.cpp Outdated Show resolved Hide resolved

andyp1per marked this pull request as ready for review February 26, 2021 22:34

andyp1per force-pushed the pr-esc-notch branch from 881fe26 to 83b0d47 Compare February 27, 2021 08:24

andyp1per force-pushed the pr-esc-notch branch from 2853df4 to 65e9b45 Compare February 27, 2021 18:43

amilcarlucas suggested changes Mar 1, 2021

View reviewed changes

libraries/AP_ESC_Telem/AP_ESC_Telem.cpp Outdated Show resolved Hide resolved

amilcarlucas reviewed Mar 1, 2021

View reviewed changes

libraries/AP_KDECAN/AP_KDECAN.cpp Outdated Show resolved Hide resolved

amilcarlucas approved these changes Mar 1, 2021

View reviewed changes

andyp1per force-pushed the pr-esc-notch branch 3 times, most recently from fc3ee8d to d6f657a Compare March 3, 2021 09:06

andyp1per requested a review from rmackay9 March 3, 2021 09:55

andyp1per added 13 commits May 7, 2021 17:13

AP_OSD: generalise ESC telemetry to allow data from other ESCs

fdb8ced

AP_PiccoloCAN: add ESC telemetry support

8127c5a

remove send_esc_telemetry_mavlink() log telemetry data in frontend record volts, amps and consumption as floats

AP_UAVCAN: add esc telemetry updates

98e477d

remove send_esc_telemetry_mavlink() remove datastructures and semaphore obsoleted by AP_ESC_Telem* (<amilcar.lucas@iav.de>) record volts, amps and consumption as floats

AP_Vehicle: generalise ESC telemetry to allow harmonic notch handling…

a81134e

… with other ESCs log ESC telemetry data in frontend

Copter: generalise ESC telemetry to allow harmonic notch handling wit…

2ef1ab7

…h other ESCs

GCS_MAVLink: use generic send_esc_telemetry_mavlink() from AP_ESC_Telem

f781a52

Plane: generalise ESC telemetry to allow harmonic notch handling with…

b50c5d9

… other ESCs

SITL: add SIM_ESC_TELEM

7eb6979

autotest: add test for ESC telemetry driven notches.

9c16039

AP_Scripting: add in ESC telemetry bindings and make optional

5300139

andyp1per force-pushed the pr-esc-notch branch from 2170e9a to fddf49b Compare May 7, 2021 16:13

andyp1per force-pushed the pr-esc-notch branch from fddf49b to 50c5ef4 Compare May 7, 2021 16:38

khancyr reviewed May 7, 2021

View reviewed changes

andyp1per removed the NeedsTesting label May 10, 2021

tridge approved these changes May 12, 2021

View reviewed changes

tridge merged commit c323ee4 into ArduPilot:master May 12, 2021

andyp1per added WikiNeeded needs wiki update and removed DevCallEU labels May 12, 2021

amilcarlucas mentioned this pull request May 19, 2021

Copter: 4.1.0-beta2 release notes #17489

Merged

Hwurzburg removed the WikiNeeded needs wiki update label May 31, 2021

tridge mentioned this pull request Mar 16, 2024

BLHeli: fixed build without BLHeli with sim on hw #26122

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generic ESC harmonic notch handling #16725

Generic ESC harmonic notch handling #16725

andyp1per commented Feb 23, 2021 •

edited

amilcarlucas commented Feb 23, 2021

amilcarlucas left a comment

amilcarlucas commented Feb 25, 2021 •

edited

amilcarlucas left a comment •

edited

andyp1per commented Feb 26, 2021

andyp1per commented Feb 27, 2021

andyp1per commented Feb 27, 2021

amilcarlucas commented Feb 28, 2021 •

edited

andyp1per commented Mar 1, 2021

amilcarlucas left a comment

amilcarlucas commented Mar 3, 2021

amilcarlucas commented Mar 3, 2021

andyp1per commented Mar 3, 2021 •

edited

andyp1per commented May 7, 2021

khancyr left a comment

rmackay9 commented May 10, 2021

tridge commented May 11, 2021

tridge commented May 12, 2021

Generic ESC harmonic notch handling #16725

Generic ESC harmonic notch handling #16725

Conversation

andyp1per commented Feb 23, 2021 • edited

amilcarlucas commented Feb 23, 2021

amilcarlucas left a comment

Choose a reason for hiding this comment

amilcarlucas commented Feb 25, 2021 • edited

amilcarlucas left a comment • edited

Choose a reason for hiding this comment

andyp1per commented Feb 26, 2021

andyp1per commented Feb 27, 2021

andyp1per commented Feb 27, 2021

amilcarlucas commented Feb 28, 2021 • edited

andyp1per commented Mar 1, 2021

amilcarlucas left a comment

Choose a reason for hiding this comment

amilcarlucas commented Mar 3, 2021

amilcarlucas commented Mar 3, 2021

andyp1per commented Mar 3, 2021 • edited

andyp1per commented May 7, 2021

khancyr left a comment

Choose a reason for hiding this comment

rmackay9 commented May 10, 2021

tridge commented May 11, 2021

tridge commented May 12, 2021

andyp1per commented Feb 23, 2021 •

edited

amilcarlucas commented Feb 25, 2021 •

edited

amilcarlucas left a comment •

edited

amilcarlucas commented Feb 28, 2021 •

edited

andyp1per commented Mar 3, 2021 •

edited