Skip to content

Core api update analog io #352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 13, 2025
Merged

Conversation

LinjingZhang
Copy link
Collaborator

@LinjingZhang LinjingZhang commented Jun 10, 2025

By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---

CONTRIBUTING.md also tells you what to expect in the PR process.

Description

  1. Add analogIO module
  2. Update the arduino-core-test
  3. Update the documentation

Related Issue
NA

Context
Test Results:
PWM Test
image
ADC Test
image
DAC Test
image

Docs Preview
image

@LinjingZhang LinjingZhang requested a review from Copilot June 10, 2025 15:17
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reintroduces and centralizes PWM channel mappings, adds full support for analog I/O (ADC and DAC) including read/write resolution, and updates core Arduino headers and tests to exercise the new analog API.

  • Restored mapping_pin_PWM4, mapping_pwm4, mapping_pin_PWM8, mapping_pwm8, and related constants to drive PWM output.
  • Implemented analogRead, analogWrite, and resolution APIs in wiring_analog.cpp and exposed prototypes in Arduino.h.
  • Extended tests in tests/test_config.h to cover analog I/O parameters and added DAC test pin definitions.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
variants/.../pins_arduino.h Un-commented and made PWM and ADC/DAC pin mappings active
tests/test_config.h Updated digital pin assignments and added analog test pins
cores/xmc/wiring_analog.cpp New implementation of analogRead/write and resolution
cores/xmc/main.cpp Removed redundant wiring_analog_init() call
cores/xmc/Arduino.h Added DEFAULT, ENABLED, DISABLED macros and analog prototype declarations
Comments suppressed due to low confidence (1)

cores/xmc/wiring_analog.cpp:133

  • Using 0b10 << _writeResolution yields 2^(resolution+1). To scale correctly by resolution bits, use (1U << _writeResolution) instead of 0b10.
uint16_t dacValue = map(value, 0, (0b10 << _writeResolution) - 1, 0, (0b10 << dac->resolution) - 1);

@@ -74,6 +74,11 @@ extern "C" {
#define interrupts() __enable_irq()
#define noInterrupts() __disable_irq()

#define DEFAULT XMC_VADC_CHANNEL_REF_INTREF
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

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

[nitpick] The macro DEFAULT is very generic and may conflict with other code. Consider renaming it to something more specific like ANALOG_REF_DEFAULT.

Suggested change
#define DEFAULT XMC_VADC_CHANNEL_REF_INTREF
#define ANALOG_REF_DEFAULT XMC_VADC_CHANNEL_REF_INTREF

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well, not easy to fix because of arduino standard and tests...

uint32_t value;

value = 0xFFFFFFFF;
if (pinNumber < NUM_ANALOG_INPUTS) {
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

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

pinNumber here is the Arduino pin index (e.g., A0 = 14), but it’s being used directly as an index into mapping_adc. You need a lookup (e.g., using scan_map_table) to map Arduino pin numbers to ADC channel entries.

Copilot uses AI. Check for mistakes.

Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
@LinjingZhang LinjingZhang force-pushed the core-api-update-analogIO branch from 7d28a21 to e5f3f64 Compare June 10, 2025 15:36
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
@@ -37,7 +37,6 @@ int main(void) {
* Initialization Time first to get closer to startup time accuracy
*/
wiring_time_init();
// wiring_analog_init();
Copy link
Member

Choose a reason for hiding this comment

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

This is no longer called anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is called in analogRead(), since it only initialize ADC

Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
@LinjingZhang LinjingZhang marked this pull request as ready for review June 13, 2025 12:53
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
Signed-off-by: zhanglinjing <Linjing.Zhang@infineon.com>
@LinjingZhang LinjingZhang force-pushed the core-api-update-analogIO branch from b6f0d9e to ac53ff2 Compare June 13, 2025 13:28
@LinjingZhang LinjingZhang merged commit 52f49d2 into core-api-update Jun 13, 2025
8 checks passed
@LinjingZhang LinjingZhang deleted the core-api-update-analogIO branch June 13, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants