Dac adc by soburi · Pull Request #27 · soburi/ArduinoCore-zephyr

soburi · 2026-03-01T01:02:39Z

No description provided.

Fixed to use %zu which corresponds to size_t. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Use a formatter to standardize indentation. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Copilot

Pull request overview

This PR updates the Zephyr-based Arduino core to add DAC-related APIs (plus ADC/PWM resolution helpers) and performs a broad formatting/cleanup pass across core files.

Changes:

Add DAC channel configuration from devicetree and a DAC-specific analogWrite(dacPins, value) overload.
Add analog read/write resolution APIs (setters and C++ getters) and apply scaling in analogRead() / PWM analogWrite().
Apply style/formatting fixes and correct printk format specifiers for size_t.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
samples/hello_arduino/src/app.cpp	Fixes `size_t` `printk` formatting (`%zu`).
cores/arduino/zephyrSerial.h	Formatting-only refactor of Serial classes/macros.
cores/arduino/zephyrSerial.cpp	Formatting-only refactor; restructures serialEvent macros/blocks.
cores/arduino/zephyrPrint.h	Formatting-only refactor of inline Print helpers.
cores/arduino/zephyrPrint.cpp	Formatting tweaks; includes a touched region with a comment typo.
cores/arduino/zephyrCommon.cpp	Adds DAC support + read/write resolution APIs; updates ADC read scaling and PWM write scaling.
cores/arduino/overloads.h	New header for DAC overload + resolution getters.
cores/arduino/main.cpp	Formatting-only refactor of main loop.
cores/arduino/apiCommon.cpp	Formatting-only refactor of wrappers.
cores/arduino/Arduino.h	Adds DAC driver include, DAC enum, and resolution API declarations/includes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-01T01:06:58Z

+static int _analog_write_resolution = 8;
+
+void analogWriteResolution(int bits) {
+	_analog_write_resolution = bits;
+}


analogWriteResolution(int bits) stores the value without validation, but later code does 1 << _analog_write_resolution. Large/negative values can trigger undefined behavior from oversized shifts and also produce incorrect scaling. Consider clamping bits to a sane range (e.g., 1..16/32 depending on platform) and using an unsigned type for the shift base (e.g., 1U/1UL).

Copilot · 2026-03-01T01:06:58Z

+	value = map(value, 0, 1 << _analog_write_resolution, 0, arduino_pwm[idx].period);

-  if (((uint32_t)value) > arduino_pwm[idx].period) {
-    value = arduino_pwm[idx].period;
-  } else if (value < 0) {
-    value = 0;
-  }
+	if (((uint32_t)value) > arduino_pwm[idx].period) {
+		value = arduino_pwm[idx].period;
+	} else if (value < 0) {
+		value = 0;
+	}


The PWM scaling uses map(value, 0, 1 << _analog_write_resolution, 0, period), which is off-by-one for typical Arduino semantics (0..(2^bits-1)). With the current code, full-scale requires passing 2^bits rather than 2^bits-1. Consider mapping from 0..((1U<<bits)-1) (and clamping inputs) so that e.g. 255 maps to 100% when bits=8.

Copilot · 2026-03-01T01:06:58Z

+	const int max_dac_value = 1U << dac_ch_cfg[dacName].resolution;
+	dac_write_value(dac_dev, dac_ch_cfg[dacName].channel_id,
+					map(value, 0, 1 << _analog_write_resolution, 0, max_dac_value));


max_dac_value is computed as 1U << resolution and used as the upper bound for map(). For an N-bit DAC, the maximum valid value is typically (1U<<N) - 1; using 1U<<N risks generating an out-of-range write at the top end. Consider using (1U<<resolution) - 1 for the max and mapping from 0..((1U<<_analog_write_resolution)-1) to 0..max.

Suggested change

const int max_dac_value = 1U << dac_ch_cfg[dacName].resolution;

dac_write_value(dac_dev, dac_ch_cfg[dacName].channel_id,

map(value, 0, 1 << _analog_write_resolution, 0, max_dac_value));

const int max_dac_value = (1U << dac_ch_cfg[dacName].resolution) - 1;

dac_write_value(dac_dev, dac_ch_cfg[dacName].channel_id,

map(value, 0, (1 << _analog_write_resolution) - 1, 0, max_dac_value));

Copilot · 2026-03-01T01:06:59Z

+#ifdef CONFIG_DAC
+
+#if (DT_NODE_HAS_PROP(DT_PATH(zephyr_user), dac))
+
+#define DAC_NODE       DT_PHANDLE(DT_PATH(zephyr_user), dac)
+#define DAC_RESOLUTION DT_PROP(DT_PATH(zephyr_user), dac_resolution)
+static const struct device *const dac_dev = DEVICE_DT_GET(DAC_NODE);
+


The DAC support is guarded by DT_NODE_HAS_PROP(..., dac) for defining dac_dev/dac_ch_cfg, but the later analogWrite(dacPins, ...) implementation is only guarded by CONFIG_DAC. If CONFIG_DAC is enabled without the devicetree 'dac' property, this will fail to compile due to dac_dev/dac_ch_cfg being undefined. Consider guarding the analogWrite implementation (and the dacPins enum) with the same DT_NODE_HAS_PROP check, or providing a stub implementation when the devicetree isn't configured.

Copilot · 2026-03-01T01:06:59Z

+// Note: We can not update the arduino_adc structure as it is read only...
+static int read_resolution = 10;
+
+void analogReadResolution(int bits) {


analogReadResolution(int bits) stores the requested bits without validation. In analogRead(), the scaling does buf << (read_resolution - seq.resolution)/>>, and very large values can cause undefined behavior due to shifting by >= width of the promoted type. Consider clamping bits to a safe range (e.g., 1..16) before storing it.

Suggested change

void analogReadResolution(int bits) {

void analogReadResolution(int bits) {

if (bits < 1) {

bits = 1;

} else if (bits > 16) {

bits = 16;

}

Copilot · 2026-03-01T01:06:59Z

+	dac_channel_setup(dac_dev, &dac_ch_cfg[dacName]);
+
+	const int max_dac_value = 1U << dac_ch_cfg[dacName].resolution;
+	dac_write_value(dac_dev, dac_ch_cfg[dacName].channel_id,
+					map(value, 0, 1 << _analog_write_resolution, 0, max_dac_value));
+}


The DAC write path ignores return values and doesn't check that the DAC device is ready. Consider checking device_is_ready(dac_dev), handling/propagating errors from dac_channel_setup()/dac_write_value(), and avoiding repeated dac_channel_setup on every write (setup once per channel if possible).

ababaec Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

69b644f Co-Authored-by: Kurt Eckhardt <kurte@rockisland.com> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Change analogReference declaration as a weak function. This is to accommodate the module configuration in the Arduino repository, but it does not have any adverse effects and is a way to coexist with the Zephyr repository, so we will include it. c8e82c1 Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

soburi added 2 commits February 27, 2026 19:26

samples: hello_arduino: Correct printk format specifier (again)

a0888d0

Fixed to use %zu which corresponds to size_t. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

cores: arduino: Format with clang-format

8f7c7db

Use a formatter to standardize indentation. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Copilot AI review requested due to automatic review settings March 1, 2026 01:02

Copilot started reviewing on behalf of soburi March 1, 2026 01:03 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

facchinm and others added 3 commits March 1, 2026 10:47

analogWrite: implement DAC pins

920891d

ababaec Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

cores: arduino: Implement analogReadResolution

abd7065

69b644f Co-Authored-by: Kurt Eckhardt <kurte@rockisland.com> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

soburi force-pushed the dac_adc branch from ba97583 to 9e0a247 Compare March 1, 2026 01:48

soburi closed this Mar 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dac adc#27

Dac adc#27
soburi wants to merge 5 commits intonextfrom
dac_adc

soburi commented Mar 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Copilot AI Mar 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-void analogReadResolution(int bits) {
+void analogReadResolution(int bits) {
+	if (bits < 1) {
+		bits = 1;
+	} else if (bits > 16) {
+		bits = 16;
+	}

Conversation

soburi commented Mar 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants