Core api update interrupt #343

dineshgit411 · 2025-05-05T06:51:53Z

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
Interrupt Module Implementation

Related Issue
NOT_AN_INTERRUPT not initialize anywhere so modified from #define digitalPinToInterrupt(p) (((p) == 9) ? 0 : NOT_AN_INTERRUPT) to #define digitalPinToInterrupt(p) (((p) == 9) ? 0 : -1)
Without digitalPinToInterrupt function, pin is not detecting in test case. so must include digitalPinToInterrupt to initialize pin.

Context
Interrupt test that are running in single board with connected jumper wire between two pins which is OUTPUT pin to INPUT pin(interrupt)

Signed-off-by: zhanglinjing <[email protected]>

Signed-off-by: MDin <[email protected]>

jaenrig-ifx

Which tests have been performed here? Against which board this is evaluated?
None of them seems to be compiling.
For this branch we have to disable in the actions check all the non-relevant boards and examples, and get this green before moving forward. Otherwise the compile-check won´t providing any helpful information.

These cover UART and Interrupts.
I would expect arduino-copre the interrupts and the UART to be all green and passing. As we don´t have this in the HIL, at least locally and place here the output.

jaenrig-ifx · 2025-05-05T07:40:52Z

cores/xmc/Uart.h

+
+    operator bool() { return true; }
+
+    void setInterruptPriority(uint32_t priority);


Are these functions public ?

hi juan, for uart we have another one branch please check other PR which is created by linjing , kindly do review also that one.

i will remove the rewrite functions and let me test again for UART implementation

Okay, i mean, if that is part of another PR it is fine.
All the uart related comments should be taken in that PR.
If this version is sufficient to get the output of the tests, that´s fine :)

jaenrig-ifx · 2025-05-05T07:41:16Z

cores/xmc/Uart.h

+    void IrqHandler(void);
+
+private:
+    RingBuffer *_rx_buffer;


Why not to have the RingBuffer object instead of a pointer?

I had the same question. Of course we can change it, but we also need to change all the declarations inside pins_arduino.h.

Unless a good reason not to be a private member of the class. Yes please.
Each UART instance handles its own buffer. The UART API public does not need to expose any of that or?

jaenrig-ifx · 2025-05-05T07:42:09Z

cores/xmc/Uart.h

+
+    size_t write(const uint8_t);
+
+    // virtual size_t write(const uint8_t *buffer, size_t size) ;


Some or all of these functions are already provided by parent classes. Unless they are different, we don´t need to rewrite them.

I can remove the inline functions, here is the reason I keep it:
I did not rewrite any function except implement write(). But the XMC4Arduino doesn't implement size_t write(const uint8_t *buffer, size_t size), just using the default implementation of Print class. But what about types other than uint8? That is why inline functions are added, it just truncates all inputs to 8-bit bytes.

If I had to severely rewrite the entire uart module, I would change this part

For all the other types you would use the print() function. And these are handled here:
https://github.com/arduino/ArduinoCore-API/blob/master/api/Print.h#L64-L91

The overloaded print() functions will eventually call write().
Why write() would need to handled that duplicating the behavior or write()?
Why do you have to rewrite the entire module?

We need to provide mandatorily:
virtual size_t write(uint8_t) = 0;
And optionally, and many times convenient:
virtual size_t write(const uint8_t *buffer, size_t size);

jaenrig-ifx · 2025-05-05T07:42:32Z

cores/xmc/Uart.h

+    int read(void);
+    void flush(void);
+
+    // virtual size_t readBytes(char *buffer, size_t length) ; // read chars from stream into buffer


Cleanup stale code or comment whey it is commented.

jaenrig-ifx · 2025-05-05T07:44:49Z

cores/xmc/Uart.h

+class Uart : public HardwareSerial {
+public:
+    XMC_UART_t *_XMC_UART_config;
+    Uart(XMC_UART_t *xmc_uart_config, RingBuffer *rx_buffer);


It would be nice if we could create a XMC_UART object by passing valid UART pins like UART(pin_tx, pin_rx, pin_cts, pin_rts).
Not sure how affordable is it to implement based on the xmc_lib API.

It should be possible. need to modify pins_arduino.h

This one probably needs more work than we want to take now. So we can create a ticket for this for the future.

jaenrig-ifx · 2025-05-05T08:29:36Z

cores/xmc/WInterrupts.c

+            interrupt_0_cb = callback;
+            NVIC_EnableIRQ(ERU0_3_IRQn);
+        } else if (pin_irq.irq_num == 1) {
+    #if defined(XMC4200_Platform2GO)


Should these be handled by defines in the pins_arduino.h ?
That way we can avoid here all these cascaded if/else that need to be modified for each board.

jaenrig-ifx · 2025-05-05T08:29:44Z

cores/xmc/WInterrupts.c

+        XMC_CCU4_EnableClock(pin_irq.ccu, pin_irq.slice_num);
+
+        if (pin_irq.irq_num == 0) {
+    #if defined(XMC1100_Boot_Kit) || defined(XMC1400_Arduino_Kit) || defined(XMC1400_XMC2GO)


Same topic...

okay, this will be evaluated in a later stage!

Signed-off-by: MDin <[email protected]>

Copilot

Pull Request Overview

This PR implements updates to the interrupt module for the XMC platform while also refactoring the UART initialization and serial event handling. Key changes include:

Updating the digitalPinToInterrupt macro to return –1 instead of NOT_AN_INTERRUPT.
Un-commenting and enabling the mapping_interrupt array and serial interrupt callback functions.
Refactoring the UART code, header inclusions, and modifying the GitHub Actions workflow configuration.

Reviewed Changes

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

Show a summary per file

File	Description
variants/XMC4700/config/XMC4700_Relax_Kit/pins_arduino.h	Updated the interrupt macro and enabled the mapping_interrupt array.
tests/test_config.h	Added a new test configuration header for board-specific test pin definitions.
cores/xmc/main.cpp	Modified the main loop to explicitly invoke ::serialEventRun() for clarity.
cores/xmc/WInterrupts.c	Introduced interrupt attach/detach functions and their callbacks.
cores/xmc/Uart.h, cores/xmc/Uart.cpp	Added new UART class implementation and initialization routines.
cores/xmc/Arduino.h	Revised includes to integrate the UART support.
.github/workflows/compile-platform-examples.yml	Commented out certain sketch compilation steps in the CI workflow.

Files not reviewed (1)

tests/arduino-core-tests: Language not supported

Comments suppressed due to low confidence (1)

variants/XMC4700/config/XMC4700_Relax_Kit/pins_arduino.h:149

[nitpick] Consider defining a named constant (e.g., INVALID_INTERRUPT) instead of using the literal -1 to improve code clarity and maintainability.

#define digitalPinToInterrupt(p) ((p) == 2 ? 0 : ((p) == 3 ? 1 : -1))

LinjingZhang · 2025-05-08T16:31:10Z

nice!
Remove the uart part in this PR if you don't want to be blocked.

Signed-off-by: MDin <[email protected]>

LinjingZhang and others added 11 commits April 15, 2025 14:42

cores/xmc/: Add Uart class for Serial implementation.

8499aa7

Signed-off-by: zhanglinjing <[email protected]>

variants/: Adapt pins_arduino.h for xmc4700.

59f347b

Signed-off-by: zhanglinjing <[email protected]>

cores/xmc: Adapt Serial after using namespace arduino.

83895b3

Signed-off-by: zhanglinjing <[email protected]>

arduino-core-tests/: Update the core test to newest main.

66d5f7e

Signed-off-by: zhanglinjing <[email protected]>

/cores/xmc/Uart.h: Fix typo.

eb65b85

Signed-off-by: zhanglinjing <[email protected]>

cores/xmc: Fix Uart bug, only use ringbuffer for rx.

5fdc74a

Signed-off-by: zhanglinjing <[email protected]>

pins_arduino.h: Update for XMC4700_Relax_Kit.

79a1e99

Signed-off-by: zhanglinjing <[email protected]>

cores/xmc/: Add Interrupt Implementation.

f39bc19

Signed-off-by: MDin <[email protected]>

variants/: Adapt pins_arduino.h for XMC4700.

03f2668

Signed-off-by: MDin <[email protected]>

tests/: Modify pin numbers for interrupts.

ca9062d

Signed-off-by: MDin <[email protected]>

tests/arduino-core-tests/: Add submodule of tests.

e3daa78

Signed-off-by: MDin <[email protected]>

dineshgit411 requested review from LinjingZhang and jaenrig-ifx May 5, 2025 06:52

jaenrig-ifx reviewed May 5, 2025

View reviewed changes

dineshgit411 added 7 commits May 7, 2025 16:28

workflows/: Comment library examples.

f11dfa0

Signed-off-by: MDin <[email protected]>

arduino-core-tests/: Update test case for interrupt.

ed08fd9

Signed-off-by: MDin <[email protected]>

compile-platform-examples: Comment library examples.

46bd28c

Signed-off-by: MDin <[email protected]>

arduino-core-tests: Add submodules.

6852bae

Signed-off-by: MDin <[email protected]>

.github/workflows: Comment library examples.

03eb83d

Signed-off-by: MDin <[email protected]>

arduino-core-tests/: Update test case for interrupt.

4343d53

Signed-off-by: MDin <[email protected]>

compile-platform-examples: Comment library examples.

654f138

Signed-off-by: MDin <[email protected]>

dineshgit411 force-pushed the core-api-update-interrupt branch from 9c7dcf8 to 654f138 Compare May 7, 2025 11:02

jaenrig-ifx mentioned this pull request May 7, 2025

Core api update serial #340

Merged

ederjc requested a review from Copilot May 7, 2025 13:18

Copilot AI reviewed May 7, 2025

View reviewed changes

jaenrig-ifx approved these changes May 8, 2025

View reviewed changes

dineshgit411 added 2 commits May 9, 2025 16:08

cores/xmc/: Removed uart files.

f004b37

Signed-off-by: MDin <[email protected]>

cores/xmc/: Comment serial functions.

7da4d7d

Signed-off-by: MDin <[email protected]>

dineshgit411 merged commit 2c0e5d4 into core-api-update May 12, 2025
18 checks passed

dineshgit411 deleted the core-api-update-interrupt branch May 12, 2025 07:20


		operator bool() { return true; }

		void setInterruptPriority(uint32_t priority);


		size_t write(const uint8_t);

		// virtual size_t write(const uint8_t *buffer, size_t size) ;

Core api update interrupt #343

Core api update interrupt #343

Uh oh!

Conversation

dineshgit411 commented May 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaenrig-ifx left a comment

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!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

LinjingZhang commented May 8, 2025

Uh oh!

Uh oh!

Uh oh!

dineshgit411 commented May 5, 2025 •

edited

Loading