sensors: add ads7953 adc #25721

Phil-Engljaehringer · 2025-10-09T15:34:42Z

Solved Problem

-Needed 16 channel ADC

Fixes #{Github issue ID}

Solution

-Write driver for ADS7953 board

Changelog Entry

-Added driver for ADS7953 board

Test coverage

-Tested performance and stability of measurements manually

Phil-Engljaehringer · 2025-10-09T15:36:41Z

Added driver for ADS7953 board

dakejahl · 2025-10-09T18:31:27Z

src/drivers/adc/ads7953/ads7953_params.c

+ * @max 3.0
+ * @decimal 2
+ */
+PARAM_DEFINE_FLOAT(ADC_ADS7953_REFV, 2.5f);


use module.yaml which is the contemporary way to define parameters
https://docs.px4.io/main/en/advanced/parameters_and_configurations.html#creating-defining-parameters

src/drivers/adc/ads7953/ADS7953.h

src/drivers/adc/ads7953/ADS7953.cpp

dakejahl · 2025-10-09T18:33:42Z

src/drivers/adc/ads7953/ADS7953.cpp

+	int ret = SPI::init();
+
+	if (ret != PX4_OK) {
+		DEVICE_DEBUG("SPI::init failed (%i)", ret);


prefer PX4_DEBUG

dakejahl · 2025-10-09T18:34:03Z

msg/AdcReport.msg

-int16[12] channel_id		# ADC channel IDs, negative for non-existent, TODO: should be kept same as array index
-int32[12] raw_data		# ADC channel raw value, accept negative value, valid if channel ID is positive
+int16[16] channel_id		# ADC channel IDs, negative for non-existent, TODO: should be kept same as array index
+int32[16] raw_data		# ADC channel raw value, accept negative value, valid if channel ID is positive


this may have implications for other drivers, did you check?

dakejahl · 2025-10-09T18:34:47Z

src/drivers/adc/ads7953/ADS7953.cpp

+	}
+
+	float ref_volt = 2.5f;
+	param_get(param_find("ADC_ADS7953_REFV"), &ref_volt);


have this class inherit ModuleParams and use _param_name.get()

Co-authored-by: Jacob Dahl <37091262+dakejahl@users.noreply.github.com>

…X4-Autopilot into pr-ads7953

alexcekay · 2025-10-13T11:47:29Z

boards/px4/fmu-v6x/default.px4board

 CONFIG_BOARD_SERIAL_TEL3="/dev/ttyS1"
 CONFIG_BOARD_SERIAL_EXT2="/dev/ttyS3"
 CONFIG_BOARD_SERIAL_RC="/dev/ttyS5"
+CONFIG_DRIVERS_ADC_ADS7953=y


We added this for testing reasons, we should remove it from v6x again, as we do not have enough FLASH

alexcekay · 2025-10-13T11:50:01Z

src/drivers/adc/ads7953/ADS7953.cpp

+}
+
+
+void ADS7953::parameters_update()


This is not called anywhere, I also do not see it required to listen for param updates as they are marked as reboot_required anyway.

alexcekay · 2025-10-13T11:50:18Z

src/drivers/adc/ads7953/ADS7953.h

+
+	static const hrt_abstime SAMPLE_INTERVAL{50_ms};
+
+	void parameters_update();


See other comment regarding parameters_udpate

alexcekay · 2025-10-13T11:53:47Z

src/drivers/adc/ads7953/ADS7953.cpp

+	}
+
+	ret = rw_msg(&recv_data[0], 0, false);
+	ret = rw_msg(&recv_data[0], 0, true);


You overwrite the previous ret, so you will mask out errors.
You can use a pattern like this:

int status = PX4_OK; status = status || <func1> status = status || <func2>

alexcekay · 2025-10-13T11:56:35Z

src/drivers/adc/ads7953/ADS7953.cpp

+int ADS7953::get_measurements()
+{
+	uint8_t recv_data[2];
+	uint8_t ch_id = 0;


use the narrowest scope, otherwise one expects that you need ch_id in different parts of the method, but you actually only need it inside the first if clause of the while loop, so its clearer if you move it there.

alexcekay · 2025-10-13T11:58:10Z

src/drivers/adc/ads7953/module.yaml

+        reboot_required: true
+        default: 0
+
+


nit: To better fit the style of other module.yaml, only have one empty line between params.

alexcekay · 2025-10-13T11:59:05Z

src/drivers/adc/ads7953/ADS7953.cpp

+	return 0;
+}
+
+


nit: You sometimes use two empty spaces between methods and sometimes one. Decide for one to keep it more consistent with others I would use one. Also check others.

alexcekay · 2025-10-13T12:01:04Z

src/drivers/adc/ads7953/ADS7953.cpp

+				mask |= (1U << ch_id);
+				count++;
+				_adc_report.channel_id[ch_id] = ch_id;
+				_adc_report.raw_data[ch_id] = ((((uint16_t) recv_data[0]) & 0x0F) << 8) | recv_data[1];  //data_value;


what should the comment //data_value tell?

alexcekay · 2025-10-13T12:17:43Z

src/drivers/adc/ads7953/ADS7953.cpp

+{
+	get_measurements();
+	_adc_report.timestamp = hrt_absolute_time();
+	_to_adc_report.publish(_adc_report);


This is outdated and does not match _adc_report_pub anymore, which causes it to not compile.

sensors: add ads7953 adc

d195ce0

dakejahl requested changes Oct 9, 2025

View reviewed changes

Phil-Engljaehringer and others added 3 commits October 10, 2025 14:02

Update src/drivers/adc/ads7953/ADS7953.h

2da1d08

Co-authored-by: Jacob Dahl <37091262+dakejahl@users.noreply.github.com>

Implemented changes suggested by review

06aa908

Merge branch 'pr-ads7953' of https://github.com/Phil-Engljaehringer/P…

edb79b7

…X4-Autopilot into pr-ads7953

Phil-Engljaehringer requested a review from dakejahl October 13, 2025 11:14

alexcekay reviewed Oct 13, 2025

View reviewed changes

Implemented suggested changes

8479db8


		static const hrt_abstime SAMPLE_INTERVAL{50_ms};

		void parameters_update();

sensors: add ads7953 adc #25721

Are you sure you want to change the base?

sensors: add ads7953 adc #25721

Conversation

Phil-Engljaehringer commented Oct 9, 2025

Solved Problem

Solution

Changelog Entry

Test coverage

Uh oh!

Phil-Engljaehringer commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants