Add AP_AngleSensor Driver with AS5048B Support, Integrate with Windvane #25749

DavidIngraham · 2023-12-11T01:33:27Z

This change includes the following:

AP_AngleSensor: a new sensor driver frontend for digital angle sensors.
AP_AngleSensor_AS5048B: An AP_AngleSensor backend for the AS5048B I2C Magnetic Encoder.
AP_WindVane_AngleSensor: An AP_WindVane backend that uses AP_AngleSensor.
AP_Vehicle Support for AngleSensor (Disabled by default for all vehicles except Rover)

While this change only integrates the driver with AP_WindVane, I expect that this driver will have future use for:

Scripting - especially for variable geometry frame configurations
Fault Detection for Tiltrotors
AOA/Sideslip sensors (see stale PR: Create simulator for AS5600 AOA sensor #16397)
Motor Position control for Mounts and End Effectors (AP_Periph based servo firmware?)
Cyclic motor control for swashplateless rotors

This was tested on a MATEK H743 Board.

IamPete1 · 2023-12-11T13:24:43Z

If the goal is to support on more than rover your better off adding it to AP_Vehicle straight away.

libraries/AP_AbsoluteEncoder/AP_AbsoluteEncoder.cpp

IamPete1 · 2023-12-11T13:30:12Z

Looks like there is some debug code that needs removing.

Hwurzburg

unless this stays Rover only and @rmackay9 agrees to have it default for that vehicle, this should be a build optioned, and probably default excluded in minimize_common

DavidIngraham · 2023-12-11T22:19:11Z

Thanks for the reviews - a summary of TODOs

Rebase onto AP_WindVane: add and use AP_WINDVANE_*_ENABLED defines #25750 and add the equivalent changes for AP_Windvane_AbsoluteEncoder.
Move instantiation out of rover into AP_Vehicle.
Wrap everything with appropriate build options.
Fix the logging bug
Reimplement instance parameters as a subtable .
Draft Wiki update and PR. Add Angle Sensor and Wind-Vane-Angle-Sensor Docs ardupilot_wiki#5650

DavidIngraham · 2023-12-12T20:09:13Z

All reviews addressed and build is passing

This creates a generic angle sensor/absolute encoder library with initial support for the AS5048B Magnetic Rotary encoder. This driver follows the typical frontend/backend pattern and future support for additional encoders is expected. This is initially integrated with AP_WindVane, but could be used anywhere that absolute angle measurements are required.

This adds a backend for AP_Windvane using the new AP_AngleSensor driver. The "DIR_PIN" param is overloaded to select the absolute encoder backend.

DavidIngraham · 2023-12-30T05:32:31Z

Rebased and Refactored to "AngleSensor" per discussion on discord.

IamPete1 · 2023-12-31T21:03:02Z

Rover/config.h

+//////////////////////////////////////////////////////////////////////////////
+// Angle Sensor Driver - useful for wind vanes
+#ifndef ANGLESENSOR_ENABLED
+  #define ANGLESENSOR_ENABLED ENABLED


Does this work? One problem with moving it to AP vehicle is that you can't see these vehicle level config files anymore. We typically have to get round it with dummy methods, which is annoying. AP_AIS would be a example.

IamPete1 · 2023-12-31T21:04:41Z

libraries/AP_AngleSensor/AP_AngleSensor.cpp

+    // @Group: _
+    // @Path: AP_AngleSensor_Params.cpp
+    AP_SUBGROUPINFO(_params[0], "_", 0, AP_AngleSensor, AP_AngleSensor_Params),


Suggested change

// @Group: _

// @Path: AP_AngleSensor_Params.cpp

AP_SUBGROUPINFO(_params[0], "_", 0, AP_AngleSensor, AP_AngleSensor_Params),

// @Group: 1_

// @Path: AP_AngleSensor_Params.cpp

AP_SUBGROUPINFO(_params[0], "1_", 1, AP_AngleSensor, AP_AngleSensor_Params),

My preference would be to number the first instance too, just keep things more consistent. Also param indexes should start at 1.

IamPete1 · 2023-12-31T21:05:27Z

libraries/AP_AngleSensor/AP_AngleSensor.cpp

+    #if ANGLE_SENSOR_MAX_INSTANCES > 1
+    // @Group: 2_
+    // @Path: AP_AngleSensor_Params.cpp
+    AP_SUBGROUPINFO(_params[1], "2_", 1, AP_AngleSensor, AP_AngleSensor_Params),
+    #endif


Suggested change

#if ANGLE_SENSOR_MAX_INSTANCES > 1

// @Group: 2_

// @Path: AP_AngleSensor_Params.cpp

AP_SUBGROUPINFO(_params[1], "2_", 1, AP_AngleSensor, AP_AngleSensor_Params),

#endif

#if ANGLE_SENSOR_MAX_INSTANCES > 1

// @Group: 2_

// @Path: AP_AngleSensor_Params.cpp

AP_SUBGROUPINFO(_params[1], "2_", 2, AP_AngleSensor, AP_AngleSensor_Params),

#endif

This moves to index 2 so we can avoid 0 on for the first set.

IamPete1 · 2023-12-31T21:07:39Z

libraries/AP_AngleSensor/AP_AngleSensor.cpp

+void AP_AngleSensor::update(void)
+{
+    for (uint8_t i=0; i<_num_instances; i++) {
+        if (drivers[i] != nullptr && _params[i]._type != ANGLESENSOR_TYPE_NONE) {


I presume the type check is so you can runtime disable? But that would just leave the state unchanged which could still be healthy?

Better to either not allow runtime disable, or if disabled then overwrite the state.

IamPete1 · 2023-12-31T21:10:17Z

libraries/AP_AngleSensor/AP_AngleSensor.cpp

+        angle_0     : (float)get_angle_radians(0),
+        quality_0   : (uint8_t)get_signal_quality(0),
+        angle_1     : (float)get_angle_radians(1),
+        quality_1   : (uint8_t)get_signal_quality(1),


Per Instance logging is much more flexible. You would add the sensor index with the magic # unit.

IamPete1 · 2023-12-31T21:14:01Z

libraries/AP_AngleSensor/AP_AngleSensor.h

+    enum AngleSensor_Type : uint8_t {
+        ANGLESENSOR_TYPE_NONE          =   ANGLE_SENSOR_TYPE_NONE,
+        ANGLESENSOR_TYPE_AS5048B       =   1,
+    };


Suggested change

enum AngleSensor_Type : uint8_t {

ANGLESENSOR_TYPE_NONE = ANGLE_SENSOR_TYPE_NONE,

ANGLESENSOR_TYPE_AS5048B = 1,

};

enum class TYPE : uint8_t {

NONE = ANGLE_SENSOR_TYPE_NONE,

AS5048B = 1,

};

Enum class's are preferred, you would then use like this: TYPE::AS5048B. Externally (its public so I guess there is a user somewhere) it would be AP_AngleSensor::TYPE::AS5048B so you still have the same descriptiveness of ANGLESENSOR_TYPE_AS5048B

IamPete1 · 2023-12-31T21:14:16Z

libraries/AP_AngleSensor/AP_AngleSensor.h

+    };
+
+    // The AngleSensor_State structure is filled in by the backend driver
+    struct AngleSensor_State {


Does this need to be public?

IamPete1 · 2023-12-31T21:15:03Z

libraries/AP_AngleSensor/AP_AngleSensor_Backend.cpp

+    _state.last_reading_ms = last_reading_ms;
+}
+
+#endif  // AP_ANGLESENSOR_ENABLED


Suggested change

#endif // AP_ANGLESENSOR_ENABLED

#endif // AP_ANGLESENSOR_ENABLED

IamPete1 · 2023-12-31T21:15:44Z

libraries/AP_AngleSensor/AP_AngleSensor_Params.cpp

+    // @Description: What type of Angle Sensor is connected
+    // @Values: 0:None,1:AS5048B
+    // @User: Standard
+    AP_GROUPINFO_FLAGS("TYPE", 0, AP_AngleSensor_Params, _type, ANGLE_SENSOR_TYPE_NONE, AP_PARAM_FLAG_ENABLE),


Per the top level, start at index 1.

IamPete1 · 2023-12-31T21:17:42Z

libraries/AP_AngleSensor/AP_AngleSensor_Params.cpp

+    // @Param: BUS
+    // @DisplayName: Angle Sensor Serial Bus Index
+    // @Description: Angle Sensor Serial Bus Index
+    // @Values: 1: I2C1, 2: I2C2, 3: I2C3
+    // @User: Standard
+    AP_GROUPINFO("BUS",    4, AP_AngleSensor_Params, _bus, ANGLE_SENSOR_BUS_DEFAULT),
+
+    // @Param: ADDR
+    // @DisplayName: Serial Bus Address
+    // @Description: Serial Bus Address
+    // @Increment: 1
+    // @User: Standard
+    AP_GROUPINFO("ADDR",    5, AP_AngleSensor_Params, _addr, ANGLE_SENSOR_ADDR_DEFAULT),


These could be in a i2c sub table that only gets loaded for this driver. As we add more types we don't want all the params all the time. A potentiometer backend would add a pin and voltage and angle pairs.

IamPete1 · 2023-12-31T21:17:48Z

libraries/AP_AngleSensor/AP_AngleSensor_Params.h

+    AP_Int8  _direction;
+};
+
+#endif // AP_ANGLESENSOR_ENABLED


Suggested change

#endif // AP_ANGLESENSOR_ENABLED

#endif // AP_ANGLESENSOR_ENABLED

IamPete1 · 2023-12-31T21:18:09Z

libraries/AP_AngleSensor/examples/AbsoluteEncoder_Test/AngleSensor_Test.cpp

@@ -0,0 +1,74 @@
+/*
+ *  RangeFinder test code


IamPete1 · 2023-12-31T21:21:53Z

libraries/AP_WindVane/AP_WindVane.cpp

@@ -49,8 +50,8 @@ const AP_Param::GroupInfo AP_WindVane::var_info[] = {

    // @Param: DIR_PIN
    // @DisplayName: Wind vane analog voltage pin for direction
-    // @Description: Analog input pin to read as wind vane direction
-    // @Values: 11:Pixracer,13:Pixhawk ADC4,14:Pixhawk ADC3,15:Pixhawk ADC6/Pixhawk2 ADC,50:AUX1,51:AUX2,52:AUX3,53:AUX4,54:AUX5,55:AUX6,103:Pixhawk SBUS
+    // @Description: Analog input pin to read as wind vane direction. If the Wind Vane type is Angle Sensor, this parameter selects the Angle Sensor Instance.


I think my preference would be to encode this in the type. Re-using pin is a bit odd. Maybe we would skip some and start at 20 for type.

"20:AngleSensor 1, 21:AngleSensor 2, 22:AngleSensor 3" ect.

Or I guess we flip it the other way from the start and have a function in the angle sensor lib. We usually endup doing that anyway when there are several instances. Servos and RC are examples, and we have just done relay.

I think I'm going to go with the first option - that's how it's done for RPM which is the closest example I can think of. I think that the function flip makes the most sense for outputs which can only be assigned one function at a time. In theory, an angle sensor could be consumed by multiple devices at a time.

IamPete1 · 2023-12-31T21:22:12Z

libraries/AP_WindVane/AP_WindVane_AngleSensor.cpp

+    }
+}
+
+#endif  //AP_WINDVANE_ANGLESENSOR_ENABLED


Suggested change

#endif //AP_WINDVANE_ANGLESENSOR_ENABLED

#endif //AP_WINDVANE_ANGLESENSOR_ENABLED

IamPete1 · 2023-12-31T21:23:01Z

libraries/AP_WindVane/AP_WindVane_AngleSensor.cpp

+    if (angle_sensor_driver != nullptr) {
+        if (angle_sensor_driver->healthy(_encoder_instance)) {


Suggested change

if (angle_sensor_driver != nullptr) {

if (angle_sensor_driver->healthy(_encoder_instance)) {

if ((angle_sensor_driver != nullptr) && (angle_sensor_driver->healthy(_encoder_instance)) {

Lazy evaluation means you can do these in one go.

IamPete1 · 2023-12-31T21:23:09Z

libraries/AP_WindVane/AP_WindVane_AngleSensor.h

+    uint8_t _encoder_instance = 0;
+};
+
+#endif  //AP_WINDVANE_ANGLESENSOR_ENABLED


Suggested change

#endif //AP_WINDVANE_ANGLESENSOR_ENABLED

#endif //AP_WINDVANE_ANGLESENSOR_ENABLED

DavidIngraham · 2024-01-02T04:43:27Z

Thanks for the reviews- working on cleaning this up now

DavidIngraham requested a review from peterbarker December 11, 2023 01:43

DavidIngraham added Rover Driver Sailboat Enhancement and removed Enhancement labels Dec 11, 2023

DavidIngraham removed the request for review from peterbarker December 11, 2023 01:49

DavidIngraham force-pushed the AP_AbsoluteEncoder branch from 3b066e2 to 6b1c917 Compare December 11, 2023 02:14

IamPete1 reviewed Dec 11, 2023

View reviewed changes

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

IamPete1 reviewed Dec 11, 2023

View reviewed changes

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

Hwurzburg added the WikiNeeded needs wiki update label Dec 11, 2023

Hwurzburg self-requested a review December 11, 2023 13:45

Hwurzburg requested changes Dec 11, 2023

View reviewed changes

DavidIngraham force-pushed the AP_AbsoluteEncoder branch 4 times, most recently from f81dafa to 92efa4a Compare December 12, 2023 06:25

DavidIngraham mentioned this pull request Dec 12, 2023

Add Angle Sensor and Wind-Vane-Angle-Sensor Docs ArduPilot/ardupilot_wiki#5650

Open

DavidIngraham force-pushed the AP_AbsoluteEncoder branch 5 times, most recently from cf78001 to 263f474 Compare December 12, 2023 18:14

DavidIngraham requested a review from Hwurzburg December 12, 2023 20:09

DavidIngraham force-pushed the AP_AbsoluteEncoder branch 2 times, most recently from d3cfcec to 5e4543d Compare December 17, 2023 16:17

Hwurzburg added the DevCallTopic label Dec 20, 2023

Hwurzburg requested a review from IamPete1 December 20, 2023 12:26

DavidIngraham added 6 commits December 29, 2023 21:20

AP_Logger: Add AngleSensor Log

ea0fa12

Tools: add AP_AngleSensor Build

acdd409

AP_Vehicle: add AngleSensor support

23b64da

AP_WindVane: Add Support for AngleSensor Backend

6f02de6

This adds a backend for AP_Windvane using the new AP_AngleSensor driver. The "DIR_PIN" param is overloaded to select the absolute encoder backend.

Rover: Enable AngleSensor by default if windvane is enabled

375e769

DavidIngraham force-pushed the AP_AbsoluteEncoder branch from 4bb2669 to 375e769 Compare December 30, 2023 05:31

DavidIngraham changed the title ~~Add AP_AbsoluteEncoder Driver with AS5048B Support, Integrate with Windvane~~ Add AP_AngleSensor Driver with AS5048B Support, Integrate with Windvane Dec 30, 2023

IamPete1 reviewed Dec 31, 2023

View reviewed changes

libraries/AP_AngleSensor/examples/AbsoluteEncoder_Test/AngleSensor_Test.cpp

@@ -0,0 +1,74 @@

/*

* RangeFinder test code

Copy link

Member

IamPete1 Dec 31, 2023

Choose a reason for hiding this comment

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

?

IamPete1 reviewed Dec 31, 2023

View reviewed changes

tridge removed the DevCallTopic label Jan 2, 2024

tekcadi mentioned this pull request Jan 24, 2024

Plane: add support for Angle of Attack (AoA) sensors #8721

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AP_AngleSensor Driver with AS5048B Support, Integrate with Windvane #25749

Add AP_AngleSensor Driver with AS5048B Support, Integrate with Windvane #25749

DavidIngraham commented Dec 11, 2023 •

edited

Loading

IamPete1 commented Dec 11, 2023

IamPete1 commented Dec 11, 2023

Hwurzburg left a comment

DavidIngraham commented Dec 11, 2023 •

edited

Loading

DavidIngraham commented Dec 12, 2023

DavidIngraham commented Dec 30, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

DavidIngraham Jan 2, 2024

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

IamPete1 Dec 31, 2023

DavidIngraham commented Jan 2, 2024

	#endif // AP_ANGLESENSOR_ENABLED
	#endif // AP_ANGLESENSOR_ENABLED

	#endif //AP_WINDVANE_ANGLESENSOR_ENABLED
	#endif //AP_WINDVANE_ANGLESENSOR_ENABLED

		if (angle_sensor_driver != nullptr) {
		if (angle_sensor_driver->healthy(_encoder_instance)) {

	if (angle_sensor_driver != nullptr) {
	if (angle_sensor_driver->healthy(_encoder_instance)) {
	if ((angle_sensor_driver != nullptr) && (angle_sensor_driver->healthy(_encoder_instance)) {

Add AP_AngleSensor Driver with AS5048B Support, Integrate with Windvane #25749

Are you sure you want to change the base?

Add AP_AngleSensor Driver with AS5048B Support, Integrate with Windvane #25749

Conversation

DavidIngraham commented Dec 11, 2023 • edited Loading

IamPete1 commented Dec 11, 2023

IamPete1 commented Dec 11, 2023

Hwurzburg left a comment

Choose a reason for hiding this comment

DavidIngraham commented Dec 11, 2023 • edited Loading

DavidIngraham commented Dec 12, 2023

DavidIngraham commented Dec 30, 2023

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

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

DavidIngraham commented Jan 2, 2024

DavidIngraham commented Dec 11, 2023 •

edited

Loading

DavidIngraham commented Dec 11, 2023 •

edited

Loading