Patch to arduino#30
Conversation
While we prepare a cleaner solution, this patch allows back and forth between pinmuxes.
Tested on UNO Q with next patch in the series and this sketch
void loop() {
Serial.begin(115200);
Serial.println("helloooooo");
delay(20);
Serial.end();
delay(20);
pinMode(1, OUTPUT);
digitalWrite(1, HIGH);
delay(10);
digitalWrite(1, LOW);
delay(100);
analogWrite(1, 33);
}
Pin 1 correctly prints the string, muxes as GPIo and then produces the required PWM
As these pins are shown as Wire pins on the official documents such as the pinout and multiple people have reported that it does not work.
Make sure the verification passed before deleting temporary files. These may be used in a re-run of verify-core if failed jobs are retried by the user via the Github interface.
added time and sys_clock_settime export Signed-off-by: Andrea Gilardoni <a.gilardoni@arduino.cc>
added ldexp symbol export
Fix minor typo in comment. Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Zephyr-based Arduino core to better handle optional DAC devicetree configuration and to improve analogWrite scaling/clamping behavior for PWM/DAC outputs.
Changes:
- Make
dac_channelsdevicetree usage conditional to avoid build failures when the property is absent. - Clamp
analogWrite()inputs before mapping to PWM/DAC hardware ranges, and initialize DAC channels only once. - Adjust Arduino API declarations for DAC enums and resolution APIs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
cores/arduino/zephyrCommon.cpp |
Adds conditional DAC channel config generation; improves PWM/DAC analogWrite() clamping/mapping; adds one-time DAC channel setup. |
cores/arduino/Arduino.h |
Makes DAC enum expansion conditional on dac_channels; moves/guards resolution API declarations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -323,34 +327,64 @@ void analogWrite(pin_size_t pinNumber, int value) { | |||
| return; | |||
| } | |||
|
|
|||
| 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) { | |||
| if (value < 0) { | |||
| value = 0; | |||
| } else if (value > maxInput) { | |||
| value = maxInput; | |||
| } | |||
|
|
|||
| const uint32_t pulse = map(value, 0, maxInput, 0, arduino_pwm[idx].period); | |||
|
|
|||
There was a problem hiding this comment.
maxInput is derived from a left-shift of _analog_write_resolution without validation. If a sketch calls analogWriteResolution(0), maxInput becomes 0 and map(value, 0, maxInput, ...) is invoked with identical in-range bounds (0..0), which is invalid for Arduino-style map() and can trigger divide-by-zero/undefined behavior. Also, large values (e.g. >= 31 on 32-bit int) make 1 << _analog_write_resolution undefined. Clamp bits in analogWriteResolution(int) to a safe range (>=1 and <= bit-width) or special-case maxInput == 0 to set pulse to 0.
| ret = dac_write_value( | ||
| dac_dev, | ||
| dac_ch_cfg[dacName].channel_id, | ||
| static_cast<uint32_t>(map(static_cast<long>(value), | ||
| 0L, | ||
| static_cast<long>(maxInput), | ||
| 0L, | ||
| static_cast<long>(max_dac_value)))); |
There was a problem hiding this comment.
ret is assigned the return value of dac_write_value(...) but never used afterward. With -Wall/-Wextra this can trigger an unused-but-set-variable warning; either drop the assignment or check/handle the error (e.g., early-return on failure).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static const struct dac_channel_cfg dac_ch_cfg[] = { | ||
| DT_FOREACH_PROP_ELEM(DT_PATH(zephyr_user), dac_channels, DAC_CHANNEL_DEFINE)}; | ||
| #if DT_NODE_HAS_PROP(DT_PATH(zephyr_user), dac_channels) | ||
| DT_FOREACH_PROP_ELEM(DT_PATH(zephyr_user), dac_channels, DAC_CHANNEL_DEFINE) | ||
| #endif | ||
| }; |
There was a problem hiding this comment.
With dac_channels absent, this initializer becomes empty and dac_ch_cfg[] will deduce a zero-length array, which is ill-formed in standard C++. To keep builds working when dac_channels isn’t present, guard the entire dac_ch_cfg definition (or provide an explicit non-zero size / alternative path) so no zero-length array is instantiated.
| ret = dac_write_value( | ||
| dac_dev, | ||
| dac_ch_cfg[dacName].channel_id, | ||
| static_cast<uint32_t>(map(static_cast<long>(value), | ||
| 0L, | ||
| static_cast<long>(maxInput), | ||
| 0L, | ||
| static_cast<long>(max_dac_value)))); |
There was a problem hiding this comment.
Similarly to the PWM path, maxInput is (1U << _analog_write_resolution) - 1U. If _analog_write_resolution is 0, maxInput becomes 0 and the subsequent map(..., 0L, maxInput, ...) will divide by zero. The DAC path should enforce a minimum resolution (or handle the 0-bit case explicitly) before calling map.
| ret = dac_write_value( | |
| dac_dev, | |
| dac_ch_cfg[dacName].channel_id, | |
| static_cast<uint32_t>(map(static_cast<long>(value), | |
| 0L, | |
| static_cast<long>(maxInput), | |
| 0L, | |
| static_cast<long>(max_dac_value)))); | |
| uint32_t dac_value; | |
| if (maxInput == 0U) { | |
| /* Avoid division by zero in map() when resolution is 0 bits */ | |
| dac_value = 0U; | |
| } else { | |
| dac_value = static_cast<uint32_t>(map(static_cast<long>(value), | |
| 0L, | |
| static_cast<long>(maxInput), | |
| 0L, | |
| static_cast<long>(max_dac_value))); | |
| } | |
| ret = dac_write_value( | |
| dac_dev, | |
| dac_ch_cfg[dacName].channel_id, | |
| dac_value); |
- 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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const int maxInput = BIT(_analog_write_resolution) - 1U; | ||
| int ret = 0; | ||
|
|
||
| if (dacName >= NUM_OF_DACS) { | ||
| return; | ||
| } | ||
|
|
||
| dac_channel_setup(dac_dev, &dac_ch_cfg[dacName]); | ||
| if (!dac_channel_initialized[dacName]) { | ||
| if (!device_is_ready(dac_dev)) { | ||
| return; | ||
| } | ||
|
|
||
| ret = dac_channel_setup(dac_dev, &dac_ch_cfg[dacName]); | ||
| if (ret != 0) { | ||
| return; | ||
| } | ||
| dac_channel_initialized[dacName] = true; | ||
| } | ||
|
|
||
| value = CLAMP(value, 0, maxInput); | ||
|
|
||
| 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 = BIT(dac_ch_cfg[dacName].resolution) - 1; | ||
| const uint32_t output = map(value, 0, maxInput, 0, max_dac_value); |
There was a problem hiding this comment.
Same issue as PWM path: maxInput = BIT(_analog_write_resolution) - 1U can be undefined if analogWriteResolution() was called with bits <= 0 or an excessively large value, and maxInput == 0 will make map(value, 0, maxInput, ...) undefined (division by zero). Please validate/clamp bits when setting _analog_write_resolution and/or guard against maxInput == 0 here before calling map().
cores: arduino: Arduino.h: Fix typo
strcpy, memcmp and memset were missing from export, they are however exported by zephyr llext/llext_export.c. Signed-off-by: Andrea Gilardoni <a.gilardoni@arduino.cc>
llext: libc: Added missing lib exports
Can be used with a class similar to
\#ifndef SPI_SLAVE_H
\#define SPI_SLAVE_H
\#include <zephyr/kernel.h>
\#include <zephyr/device.h>
\#include <zephyr/init.h>
\#include <zephyr/drivers/spi.h>
\#ifdef CONFIG_BOARD_ARDUINO_UNO_Q
\#define SPI_SLAVE_NODE DT_COMPAT_GET_ANY_STATUS_OKAY(zephyr_spi_slave)
template <int SPI_MAX_MESSAGE>
class SPISlaveClass {
public:
SPISlaveClass() {
spi_cfg.frequency = 1000000;
spi_cfg.operation = SPI_WORD_SET(8) | SPI_OP_MODE_SLAVE;
rx.buf = rxmsg;
rx.len = SPI_MAX_MESSAGE;
rx_bufs.buffers = ℞
rx_bufs.count = 1;
tx.buf = txmsg;
tx.len = SPI_MAX_MESSAGE;
tx_bufs.buffers = &tx;
tx_bufs.count = 1;
}
int begin() {
int ret = device_init(spi_slave);
return ret;
}
void* populate(uint8_t* buf, size_t len) {
return memcpy(tx.buf, buf, len);
}
int ready() {
return spi_transceive(spi_slave, &spi_cfg, &tx_bufs, &rx_bufs);
}
private:
const struct device *const spi_slave = DEVICE_DT_GET(DT_BUS(SPI_SLAVE_NODE));
struct spi_config spi_cfg;
uint8_t rxmsg[SPI_MAX_MESSAGE];
struct spi_buf rx ;
struct spi_buf_set rx_bufs;
uint8_t txmsg[SPI_MAX_MESSAGE];
struct spi_buf tx ;
struct spi_buf_set tx_bufs;
};
\#endif
\#endif //SPI_SLAVE_H
Sketch ->
SPISlaveClass<512> spi;
void setup() {
spi.begin();
}
int i = 0;
void loop() {
spi.populate((uint8_t*)&i, 4);
spi.ready();
i++;
}
Linux side ->
pip install spidev
sudo chown arduino /dev/spidev0.0
python3
import spidev
spi = spidev.SpiDev()
spi.open_path("/dev/spidev0.0")
spi.max_speed_hz = 20000000
spi.readbytes(512)
IMPORTANT: you must always read the full buffer size (declared in the template instantiation)
llext: adding missing time and math functions export
This change fixes the errors that are raised when the workflow is triggered by a push event, which does not have a PR number associated with it. Also, no need to error if the PR number is invalid or if the workflow file is changed in the PR - just issue a warning. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
ci/package_core: cleanup test results only on positive result
Nicla Vision: initial variant support
wire: unclaim ringbuffer space on read error
core: arduino: add ShiftIn/ShiftOut implementation
Switch to using the tool binaries generated by CI instead of the ones generated manually. No functional change in the tool sources. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
add empty files: wiring_private.h pins_arduino.h resolves: #393 Also mentioned in a pull request against one of the Adafruit libraries: adafruit/Adafruit_ILI9341#103 Which has not been touched for several months.
fix: analogWrite() silently fails when PWM pin lookup fails on Uno Q
The regex-based size calculation using arm-zephyr-eabi-size could not distinguish between static and dynamic link modes, nor account for CONFIG_LLEXT_RODATA_NO_RELOC keeping .rodata sections in flash. Define a proper tool: zephyr-check-size Signed-off-by: Gilberto Conti <g.conti@arduino.cc>
tools: replace arm-zephyr-eabi-size with zephyr-check-size
Co-authored-by: Gilberto Conti <g.conti@arduino.cc> Signed-off-by: Gilberto Conti <g.conti@arduino.cc>
tools: add bin2uf2 tool
Add a new GitHub Actions workflow to generate a full package index file from the CI-generated JSON snippet and upload it to GitHub Pages after a successful run of the "Package, test and upload core" workflow on the main branch. Signed-off-by: Luca Burelli <l.burelli@arduino.cc>
ci: add JSON upload to GitHub Pages workflow
artifacts: use ci-generated tool binaries
Signed-off-by: Daniele Cloralio <d.cloralio@arduino.cc>
core: arduino: add common files for compatibility
core: llext: export workqueue symbols
Signed-off-by: Daniele Cloralio <d.cloralio@arduino.cc>
unoq: fix for analogWrite
core: arduino: analogWrite: Fix DAC output calculation
CAN: add library and overlay configuration
zephyrCommon: fixs analogWrite overflow
…ecycle Rework tone/noTone internals to better match Arduino behavior while making concurrent tone usage configurable and safer. - Add CONFIG_ARDUINO_MAX_TONES (default: -1). - Negative values fall back to the digital pin count from devicetree. - Replace per-pin tone timer arrays with slot-based pin_timer entries. - Remove the timeout companion timer and finish finite tones by counting remaining toggles in tone_expiry_cb(). Signed-off-by: TOKITA Hiroshi <tokita.hiroshi@gmail.com>
soburi
force-pushed
the
patch_to_arduino
branch
from
0795a5f to
38c4714
Compare
No description provided.