Improve analogRead() and add DAC support by soburi · Pull Request #175 · zephyrproject-rtos/ArduinoCore-zephyr

soburi · 2026-04-17T12:50:13Z

No description provided.

Fixes compilation error found at https://www.cnx-software.com/2024/12/10/arduino-core-for-zephyr-beta-released/ Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

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

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

Take into account the analog switch Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

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

Formatting ADC/DAC codes. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

- Remove duplicated `analogReadResolution` declaration - Add the same #ifdef conditions used in the implementation to the `analogWriteResolution` as well. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

- Fixed input max value calculation (Needs to be -1 after shift) - Clamp calculations are performed first, eliminating cast Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

- Correct max value calculation - Add guard for DAC entry is not defined case - initialize only if DAC not initialized Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Co-Authored-by: Giovanni Bruno <g.bruno@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Explicitly state the license in all source files to comply with SPDX best practices. In addition, update the copyright notice to the new format. Note: At the time of cherry-picking, this was applied only to cores/arduino/time_macros.h and cores/arduino/overloads.h. Co-Authored-by: Luca Burelli <l.burelli@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

Copilot

Pull request overview

This PR extends the Zephyr-based Arduino core to improve analog I/O behavior by adding configurable read/write resolutions, scaling PWM output to match the configured resolution, and introducing initial DAC support, along with a small set of Arduino-compatible time helper macros.

Changes:

Add DAC device/channel support and a DAC-specific analogWrite() overload.
Add analogReadResolution() / analogWriteResolution() APIs and scale analogRead()/PWM output accordingly.
Introduce time_macros.h and overloads.h to provide additional Arduino-compatibility APIs for C++ builds.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
cores/arduino/zephyrCommon.cpp	Adds DAC support, read/write resolution handling, PWM scaling, and analog read scaling logic.
cores/arduino/time_macros.h	Adds Arduino-style clock cycle/time conversion macros using Zephyr time utilities.
cores/arduino/overloads.h	Declares C++ overloads/getters (DAC analogWrite overload and resolution getters).
cores/arduino/Arduino.h	Exposes new APIs/types (DAC pins enum, analog read/write resolution), and includes new headers.

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

Copilot · 2026-04-17T22:35:27Z

+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+


overloads.h is a header but has no include guard / #pragma once. This can lead to repeated processing and makes it inconsistent with the rest of the core headers (which all use #pragma once). Add an include guard or #pragma once at the top.

Suggested change

#pragma once

Copilot · 2026-04-17T22:35:28Z

+// In c++ mode, we also provide analogReadResolution and analogWriteResolution getters
+int analogReadResolution();
+int analogWriteResolution();


analogReadResolution() and analogWriteResolution() getters are declared unconditionally, but their definitions are compiled only when CONFIG_ADC and CONFIG_PWM || CONFIG_DAC are enabled respectively. This can cause undefined references for C++ sketches built without those Kconfig options. Wrap these declarations in matching #if guards (or provide always-available stubs that return a default).

Suggested change

// In c++ mode, we also provide analogReadResolution and analogWriteResolution getters

int analogReadResolution();

int analogWriteResolution();

// In c++ mode, we also provide analogReadResolution and analogWriteResolution getters

#ifdef CONFIG_ADC

int analogReadResolution();

#endif

#if defined(CONFIG_PWM) || defined(CONFIG_DAC)

int analogWriteResolution();

#endif

Copilot · 2026-04-17T22:35:28Z

+#define clockCyclesPerMicrosecond()  (1000000 / k_cyc_to_ns_near64(1000))
+#define clockCyclesToMicroseconds(a) (a / clockCyclesPerMicrosecond())
+#define microsecondsToClockCycles(a) (a * clockCyclesPerMicrosecond())


The clockCyclesToMicroseconds(a) / microsecondsToClockCycles(a) macros don't parenthesize a, so expressions like x + 1 will be evaluated with unexpected precedence. Also clockCyclesPerMicrosecond() can evaluate to 0 on low-frequency targets due to integer division, which would make clockCyclesToMicroseconds() divide by zero. Parenthesize macro arguments and consider computing cycles/us with 64-bit math and rounding so it never returns 0.

Suggested change

#define clockCyclesPerMicrosecond() (1000000 / k_cyc_to_ns_near64(1000))

#define clockCyclesToMicroseconds(a) (a / clockCyclesPerMicrosecond())

#define microsecondsToClockCycles(a) (a * clockCyclesPerMicrosecond())

#define clockCyclesPerMicrosecond() ((1000000ULL + k_cyc_to_ns_near64(1000) - 1) / k_cyc_to_ns_near64(1000))

#define clockCyclesToMicroseconds(a) ((a) / clockCyclesPerMicrosecond())

#define microsecondsToClockCycles(a) ((a) * clockCyclesPerMicrosecond())

Copilot · 2026-04-17T22:35:28Z

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


analogWriteResolution(int bits) accepts any value, but later BIT(_analog_write_resolution) is used to compute maxInput. If bits <= 0 this yields maxInput == 0 and causes a division-by-zero in map64(). If bits is negative or too large, the shift inside BIT() is undefined. Clamp/validate bits to a safe range (e.g., 1..31) and consider storing it as an unsigned type.

Suggested change

_analog_write_resolution = bits;

_analog_write_resolution = CLAMP(bits, 1, 31);

Copilot · 2026-04-17T22:35:28Z

+#ifdef CONFIG_DAC
+void analogWrite(enum dacPins dacName, int value) {
+#if DT_PROP_LEN_OR(DT_PATH(zephyr_user), dac_channels, 0) > 0
+	const int maxInput = BIT(_analog_write_resolution) - 1U;
+	int ret = 0;
+
+	if (dacName >= NUM_OF_DACS) {
+		return;
+	}


Adding analogWrite(enum dacPins, int) introduces an overload that can make existing calls like analogWrite(3, value) ambiguous on C++ (an int can convert to both pin_size_t and an unscoped enum). Consider avoiding the overload (e.g., use pin_size_t for DAC pins too, or provide a differently named API / a scoped enum class with explicit conversion).

Copilot · 2026-04-17T22:35:28Z

@@ -395,9 +472,20 @@ void analogReference(uint8_t mode) {
 	}


analogReference() updates the separate channel_cfg[] array, but analogRead() still calls adc_channel_setup() with &arduino_adc[idx].channel_cfg. That means changes made by analogReference() are never applied to actual reads. Use channel_cfg[idx] in adc_channel_setup() (and ensure the indexing matches), or update arduino_adc[idx].channel_cfg instead of maintaining a separate array.

Copilot · 2026-04-17T22:35:29Z

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


analogReadResolution(int bits) stores bits directly, but analogRead() later shifts by (seq.resolution - read_resolution) / (read_resolution - seq.resolution). If bits is negative or very large, this can trigger undefined shifts and/or signed overflow in the return expression. Validate/clamp bits to a sensible range (e.g., 1..16 or 1..31) and do the scaling using an unsigned wider type before converting to int.

Copilot · 2026-04-17T22:35:29Z

+	 * number of bits the user has asked for
+	 */
+	if (read_resolution == seq.resolution) {
+		return buf;
+	}
+	if (read_resolution < seq.resolution) {
+		return buf >> (seq.resolution - read_resolution);
+	}
+	return buf << (read_resolution - seq.resolution);


The scaling logic in analogRead() does shifts based on user-controlled read_resolution. Without clamping, buf >> (seq.resolution - read_resolution) / buf << (read_resolution - seq.resolution) can shift by a negative or excessively large amount, and the left-shift is done on an int after promotion which can overflow (UB). Clamp read_resolution to a safe range and perform the shift on a wider unsigned type before returning.

Suggested change

* number of bits the user has asked for

*/

if (read_resolution == seq.resolution) {

return buf;

}

if (read_resolution < seq.resolution) {

return buf >> (seq.resolution - read_resolution);

}

return buf << (read_resolution - seq.resolution);

* number of bits the user has asked for.

*

* Clamp the user-controlled resolution to a range that is safe for

* shifting a 32-bit temporary and that still fits in the positive

* range of the int return type.

*/

int effective_read_resolution = read_resolution;

if (effective_read_resolution < 0) {

effective_read_resolution = 0;

} else if (effective_read_resolution > 31) {

effective_read_resolution = 31;

}

if (effective_read_resolution == seq.resolution) {

return buf;

}

if (effective_read_resolution < seq.resolution) {

return static_cast<int>(static_cast<uint32_t>(buf) >>

(seq.resolution - effective_read_resolution));

}

return static_cast<int>(static_cast<uint32_t>(buf)

<< (effective_read_resolution - seq.resolution));

soburi force-pushed the update_analog branch from ea1d689 to d43e5d0 Compare April 17, 2026 21:13

soburi changed the title ~~Update analog~~ Improve analogRead() and add DAC support Apr 17, 2026

core: add time related macros

1fafd6c

Fixes compilation error found at https://www.cnx-software.com/2024/12/10/arduino-core-for-zephyr-beta-released/ Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

soburi force-pushed the update_analog branch from d43e5d0 to ffb5223 Compare April 17, 2026 22:06

facchinm and others added 11 commits April 18, 2026 07:12

analogWrite: implement DAC pins

f402b2e

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

Implement analogReadResolution

5bd7455

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

unoq: implement analogReference

30f567a

Take into account the analog switch Co-Authored-by: Martino Facchin <m.facchin@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

make internal arduino_adc array static

dc63ec6

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

ZephyrCommon: allow boards without any ADC channel configured

698fb2e

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

cores: arduino: Format ADC/DAC part with clang-format

ce9649a

Formatting ADC/DAC codes. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

cores: arduino: Improve analog(Read|Write)Rsolution declaration

88525eb

- Remove duplicated `analogReadResolution` declaration - Add the same #ifdef conditions used in the implementation to the `analogWriteResolution` as well. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

cores: arduino: analogWrite: Fix max value calculation

bd1fd92

- Fixed input max value calculation (Needs to be -1 after shift) - Clamp calculations are performed first, eliminating cast Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

cores: arduino: analogWrite: Fix DAC output calculation

bebe022

- Correct max value calculation - Add guard for DAC entry is not defined case - initialize only if DAC not initialized Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

zephyrCommon: fixs analogWrite overflow

b1ceda7

Co-Authored-by: Giovanni Bruno <g.bruno@arduino.cc> Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>

soburi force-pushed the update_analog branch from ffb5223 to a28be06 Compare April 17, 2026 22:14

soburi marked this pull request as ready for review April 17, 2026 22:29

soburi requested review from DhruvaG2000 and Copilot April 17, 2026 22:29

Copilot started reviewing on behalf of soburi April 17, 2026 22:29 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve analogRead() and add DAC support#175

Improve analogRead() and add DAC support#175
soburi wants to merge 12 commits intozephyrproject-rtos:nextfrom
soburi:update_analog

soburi commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	_analog_write_resolution = bits;
	_analog_write_resolution = CLAMP(bits, 1, 31);

-	 * number of bits the user has asked for
-	 */
-	if (read_resolution == seq.resolution) {
-		return buf;
-	}
-	if (read_resolution < seq.resolution) {
-		return buf >> (seq.resolution - read_resolution);
-	}
-	return buf << (read_resolution - seq.resolution);
+	 * number of bits the user has asked for.
+	 *
+	 * Clamp the user-controlled resolution to a range that is safe for
+	 * shifting a 32-bit temporary and that still fits in the positive
+	 * range of the int return type.
+	 */
+	int effective_read_resolution = read_resolution;
+	if (effective_read_resolution < 0) {
+		effective_read_resolution = 0;
+	} else if (effective_read_resolution > 31) {
+		effective_read_resolution = 31;
+	}
+	if (effective_read_resolution == seq.resolution) {
+		return buf;
+	}
+	if (effective_read_resolution < seq.resolution) {
+		return static_cast<int>(static_cast<uint32_t>(buf) >>
+							   (seq.resolution - effective_read_resolution));
+	}
+	return static_cast<int>(static_cast<uint32_t>(buf)
+						   << (effective_read_resolution - seq.resolution));

Conversation

soburi commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants