AHRS: add ability to configure AHRS via mavlink commands #28485

valbv · 2024-10-28T10:27:03Z

It's for the ability to send commands to EAHRS like "Enable/Disable GNSS", "Start/Stop magnitometer calibration in flight" and everything that device can receive

Ryanf55 · 2024-10-28T10:39:05Z

Nice. If you can, feel free to add one such packet so that this code is used by a driver.
We prefer all code is used.

valbv · 2024-11-29T19:20:53Z

Linked PR in MAVLink repo:
ArduPilot/mavlink#376

peterbarker

I'd prefer to have something at the AHRS-backend level rather than at the EAHRS level for this. Turning GNSS off is a thing for EKF3, for example, just not in master right now.

valbv · 2024-12-10T02:54:25Z

@peterbarker , where can I find the code you were talking about (Turning GNSS off for EKF3)?
I want to avoid conflicts in the names of methods.

I looked through the branches in the repository and requests, but I couldn't find it

Ryanf55 · 2024-12-10T06:19:36Z

Check the code path of aux function 65 "GPS Disable": https://ardupilot.org/copter/docs/common-auxiliary-functions.html

valbv · 2024-12-15T23:40:36Z

Did I get the idea right, you want to see something like this?

libraries/AP_AHRS/AP_AHRS.h

libraries/AP_ExternalAHRS/AP_ExternalAHRS.h

Ryanf55 · 2024-12-17T05:46:30Z

yea, this is nice and portable between the EARHS drivers. Just making sure for commands we have a way to undo them (ie turn GNSS on/off in flight to test dead reckoning)

valbv · 2024-12-17T22:39:06Z

Yes, we used set_gnss_state method-logic for the dead reckoning tests with EAHRS

Ryanf55

Nice, yea I'm overall on board with the approach here. I haven't tested it yet.

Ryanf55 · 2024-12-18T05:31:10Z

libraries/AP_AHRS/AP_AHRS.cpp

+#if AP_AHRS_EXTERNAL_ENABLED
+    case EKFType::EXTERNAL:
+        return external.set_gnss_state(enabled);
+#endif


The code paths that aux function 105 takes could be used to disable GPS in EKF2/3.
105

Not required for the PR.

Ok, I'll create a next request with these changes

Ok, I'll create a next request with these changes

... not sure I understand this response.

We have an existing mechanism for enabling/disabling gnss use, we should use it here.

I'm not sure that I understood what I need right, but I added the GPS disabling for EKF2/3

libraries/GCS_MAVLink/GCS_Common.cpp

peterbarker · 2024-12-23T02:45:47Z

This is looking nicer than the write-bytes approach.

I'm going to mark this for DevCall for discussion.

Ryanf55 · 2024-12-23T23:17:21Z

Nothing to add from me, I'm in favor of this change and believe it sets up a nice pattern for adding future generic commands to the EKF/EAHRS.

peterbarker · 2024-12-23T02:18:09Z

libraries/AP_AHRS/AP_AHRS.cpp

+#if AP_AHRS_EXTERNAL_ENABLED
+    case EKFType::EXTERNAL:
+        return external.set_gnss_state(enabled);
+#endif


Ok, I'll create a next request with these changes

... not sure I understand this response.

We have an existing mechanism for enabling/disabling gnss use, we should use it here.

peterbarker · 2024-12-23T02:18:58Z

libraries/AP_ExternalAHRS/AP_ExternalAHRS.cpp

+bool AP_ExternalAHRS::write_bytes(const char *bytes, uint8_t len)
+{
+    if (!backend) {
+        return false;
+    }
+    return backend->write_bytes(bytes, len);
+}
+


No longer needed

peterbarker · 2024-12-23T02:19:05Z

libraries/AP_ExternalAHRS/AP_ExternalAHRS.h

@@ -121,6 +121,9 @@ class AP_ExternalAHRS {
    bool get_accel(Vector3f &accel);
    void send_status_report(class GCS_MAVLINK &link) const;
    bool get_variances(float &velVar, float &posVar, float &hgtVar, Vector3f &magVar, float &tasVar) const;
+    void set_data_sending_state(bool enabled);
+    void set_gnss_state(bool enabled);
+    bool write_bytes(const char *bytes, uint8_t len);


peterbarker · 2024-12-23T02:20:18Z

libraries/AP_ExternalAHRS/AP_ExternalAHRS_InertialLabs.h

@@ -220,6 +220,9 @@ class AP_ExternalAHRS_InertialLabs : public AP_ExternalAHRS_backend {
    void update_thread();
    bool check_uart();
    bool check_header(const ILabsHeader *h) const;
+    bool write_bytes(const char *bytes, uint8_t len) override;


No longer an override; should be private

peterbarker · 2024-12-23T02:20:39Z

libraries/AP_ExternalAHRS/AP_ExternalAHRS_backend.h

@@ -42,6 +42,9 @@ class AP_ExternalAHRS_backend {
    virtual bool pre_arm_check(char *failure_msg, uint8_t failure_msg_len) const = 0;
    virtual void get_filter_status(nav_filter_status &status) const {}
    virtual bool get_variances(float &velVar, float &posVar, float &hgtVar, Vector3f &magVar, float &tasVar) const { return false; }
+    virtual void set_data_sending_state(bool enabled) {}
+    virtual void set_gnss_state(bool enabled) {}
+    virtual bool write_bytes(const char *bytes, uint8_t len) { return false; }


Suggested change

virtual bool write_bytes(const char *bytes, uint8_t len) { return false; }

peterbarker · 2024-12-23T02:21:05Z

libraries/GCS_MAVLink/GCS_Common.cpp

+{
+    switch (packet.command) {
+        case MAV_CMD_AHRS_START:
+        {


Suggested change

{

etc.

rmackay9 · 2024-12-23T23:25:04Z

libraries/AP_ExternalAHRS/AP_ExternalAHRS_InertialLabs.cpp

+
+void AP_ExternalAHRS_InertialLabs::set_data_sending_state(bool enabled)
+{
+    if (enabled)


nitpick: bracket should be on the same line as the if

rmackay9 · 2024-12-23T23:27:28Z

Hi @valbv,

Thanks for this. What's the purpose of disabling an EAHRS? I guess it's to test a complete failure of the external AHRS system? I wonder what testing has been done for this?

rmackay9 · 2024-12-23T23:29:44Z

libraries/AP_AHRS/AP_AHRS_External.cpp

@@ -137,4 +137,14 @@ void AP_AHRS_External::get_control_limits(float &ekfGndSpdLimit, float &ekfNavVe
    ekfNavVelGainScaler = 0.5;
 }

+void AP_AHRS_External::set_data_sending_state(bool enabled)


let's put some comments above the methods..

To elaborate, at minimum, copy the comments across all the headers. Although the clasess inherit/override methods, not everyone's editor propogates the description, so AP generally copies comments.

rmackay9 · 2024-12-23T23:29:57Z

libraries/AP_AHRS/AP_AHRS_External.h

@@ -87,6 +87,9 @@ class AP_AHRS_External : public AP_AHRS_Backend {
    void send_ekf_status_report(class GCS_MAVLINK &link) const override;

    void get_control_limits(float &ekfGndSpdLimit, float &controlScaleXY) const override;
+
+    void set_data_sending_state(bool enabled) override;


comments here too would be nice

rmackay9 · 2024-12-23T23:31:36Z

More comments would be nice, there are very few which makes it difficult to review but also it will be even more difficult for future developers trying to figure out how to use the various methods

Ryanf55 added Enhancement ExternalAHRS labels Oct 28, 2024

valbv marked this pull request as draft October 28, 2024 10:40

valbv force-pushed the eahrscommands branch 3 times, most recently from b19d652 to c4ca633 Compare November 29, 2024 19:14

valbv mentioned this pull request Nov 29, 2024

common.xml: Add AHRS commands ArduPilot/mavlink#376

Open

valbv marked this pull request as ready for review November 29, 2024 19:21

valbv force-pushed the eahrscommands branch 5 times, most recently from 5f30de1 to 8faccc4 Compare December 5, 2024 18:57

peterbarker reviewed Dec 8, 2024

View reviewed changes

valbv force-pushed the eahrscommands branch from 8faccc4 to 0efa597 Compare December 15, 2024 23:31

valbv requested a review from peterbarker December 17, 2024 05:26

Ryanf55 reviewed Dec 17, 2024

View reviewed changes

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

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

valbv force-pushed the eahrscommands branch from 0efa597 to 7f58663 Compare December 17, 2024 22:33

valbv requested a review from Ryanf55 December 17, 2024 22:39

Ryanf55 approved these changes Dec 18, 2024

View reviewed changes

peterbarker added DevCallTopic WIP labels Dec 23, 2024

peterbarker requested changes Dec 23, 2024

View reviewed changes

CraigElder removed the DevCallTopic label Dec 23, 2024

rmackay9 reviewed Dec 23, 2024

View reviewed changes

peterbarker changed the title ~~ExternalAHRS: Add the ability to send commands to the EAHRS~~ AHRS: add ability to configure AHRS via mavlink commands Dec 23, 2024

valbv force-pushed the eahrscommands branch from 7f58663 to 7779d9d Compare December 25, 2024 01:47

valbv requested review from peterbarker and rmackay9 December 26, 2024 17:42

Valentin Bugrov added 5 commits December 26, 2024 16:50

ExternalAHRS: Add the ability to send commands to the EAHRS

0c58362

RC_Channel: update EAHRS method name

8c16b92

AP_AHRS: Add GNSS and data sending disable/enable

eed9c17

GCS_MAVLink: Add EAHRS commands handling

3d32630

MAVLink: Add EAHRS commands

60d24f3

valbv force-pushed the eahrscommands branch from 7779d9d to 60d24f3 Compare December 26, 2024 21:51

AHRS: add ability to configure AHRS via mavlink commands #28485

Are you sure you want to change the base?

AHRS: add ability to configure AHRS via mavlink commands #28485

Conversation

valbv commented Oct 28, 2024

Ryanf55 commented Oct 28, 2024 • edited Loading

valbv commented Nov 29, 2024

peterbarker left a comment

Choose a reason for hiding this comment

valbv commented Dec 10, 2024

Ryanf55 commented Dec 10, 2024

valbv commented Dec 15, 2024

Ryanf55 commented Dec 17, 2024

valbv commented Dec 17, 2024

Ryanf55 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbarker commented Dec 23, 2024

Ryanf55 commented Dec 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 commented Dec 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmackay9 commented Dec 23, 2024

Ryanf55 commented Oct 28, 2024 •

edited

Loading