Skip to content
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

Enable XMC1400 Kit for Arduino board into the Arduino IDE #190

Merged
merged 17 commits into from
Jul 4, 2022

Conversation

boramonideep
Copy link
Collaborator

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
This adds the XMC 1400 Kit for Arduino board into the XMC for Arduino BSP

Related Issue
#189 on github

Context
Please refer to internal ticket https://jirard.intra.infineon.com/browse/DESMAKERS-2490

@boramonideep boramonideep changed the title Enable xmc1400board 2 Enable XMC1400 Kit for Arduino board into the Arduino IDE Jun 29, 2022
@boramonideep boramonideep self-assigned this Jun 29, 2022
@techpaul
Copy link
Contributor

techpaul commented Jun 29, 2022

IN general good suggest a modification for simplicity now and in future maintainability.

CURRENTLY only the XMC1100 and XMC1400 Boot Kits have a GPIO pin assigned for Reset interrupt routing, instead of continually modifying core files, try adding TWO DEFINES to the pins_arduino.h for each of these boards

#define HAS_GPIO_RESET
#define GPIO_RESET_PIN

Change Main.cpp and reset.c (and any other place to use

#ifdef HAS_GPIO_RESET

Use GPIO_RESET_PIN to access which port and pin to use for any operations if needed now or in future..

This simplifies readablity and maintainability of code.

While at it remove from reset.c

#include <reset.h>

add instead

#include <Arduino.h>

In reset.h remove

#include <Arduino.h>

This removes circular fetches and reset.h is mainly referenced in Arduino.h anyway.

Basically .cpp and .c files should include <Arduino.h> where as .h files should NOT reference it to avoid circular references and unnecessary fetches and disk activity. Majority of cores .h files are included by Arduino.h .

Anything that calls any cores .h file should be called from Arduino.h or from a .c/.cpp module that has ALREADY included Arduino.h FIRST or after gcc standard includes

@techpaul
Copy link
Contributor

Has the information been put together for the addition to the Wiki as well ?

Copy link
Contributor

@techpaul techpaul left a comment

Choose a reason for hiding this comment

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

See comments in discussion and on files

Copy link
Contributor

@techpaul techpaul left a comment

Choose a reason for hiding this comment

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

Further issues noted see my wiki on this and read it carefully including suggested pin number
https://github.com/techpaul/XMC-for-Arduino/wiki/XMC1400-Arduino-Kit-Pinout-and-Issues

cores/WInterrupts.c Outdated Show resolved Hide resolved
Copy link
Member

@jaenrig-ifx jaenrig-ifx left a comment

Choose a reason for hiding this comment

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

I think all these pins potential issues are due to the typos in documentation.
All rest looks fine to me prior on target testing 👍

libraries/Wire/src/utility/xmc_i2c_conf.h Show resolved Hide resolved
@techpaul
Copy link
Contributor

@jaenrig-ifx @boramonideep

I have a physical board here and I would not even attempt at testing here.

Side note please do not use INTERNAL resources to Infineon on Github as per initial pull request.

Copy link
Contributor

@djaumann djaumann left a comment

Choose a reason for hiding this comment

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

I checked the hardware as well and did not have any issues, only the mismatching of the silkscreen, which you've addressed already and the comment I left you regarding the AD_AUX or AUX_AD

variants/XMC1400/config/XMC1400_Arduino_Kit/pins_arduino.h Outdated Show resolved Hide resolved
@techpaul
Copy link
Contributor

techpaul commented Jul 4, 2022

@boramonideep @jaenrig-ifx
All your fixes are BACK TO FRONT

To follow the Hardware and Software Arduino pin referenceing WHATEVER the ID says Pin 2 and 3 MUST be interrupts NOT arduino pin 26.

This means om ANY board with the same connectors especially the XMC boards the same shield and SOFTWARE will work the same WITHOUT MODIFICATION !!

By having interrupt on Arduino pin 26 you have BROKEN the Arduino Uno Shieled R3 compatability. The idents are the LEAST of your worries. The pin order in software to match order on connectors matter regardless of ident.

You change the definition of pin 2 and 3 to match what SHOULD happen NOT the ident or amy other weird method.

In Arduino NOBODY use P1.4 referencing, they use Arduino pin wiring definitions

So the ident is wrong, you obviously understand little about the arduino structure

I also agree with @djaumann ) about putting comments about AD_AUX1 to AD_AUX_n

Also as commented above you need to instigate a board agnostic method of reset pin handling as in

#define HAS_GPIO_RESET
#define GPIO_RESET_PIN

This is a minimalistic approach to adding support, as in it works if you stand on one leg with your arm behind your back singing "Yankee doodle dandy"

Copy link
Contributor

@techpaul techpaul left a comment

Choose a reason for hiding this comment

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

Still things BROKEN and still things that need better implementation

cores/Main.cpp Show resolved Hide resolved
@@ -26,7 +26,7 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <reset.h>
#ifdef XMC1100_Boot_Kit
#if defined(XMC1100_Boot_Kit) || defined(XMC1400_Arduino_Kit)
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to board agnostic method here using define in pins_arduino.h (also on XMC1100 Boot Kit)

#define HAS_GPIO_RESET

so
#ifdef HAS_GPIO_RESET

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as before when you change from ONE to more you put in a better solution AT THAT point

libraries/SPI/src/utility/xmc_spi_conf.h Show resolved Hide resolved
libraries/Wire/src/utility/xmc_i2c_conf.h Show resolved Hide resolved
variants/XMC1400/config/XMC1400_Arduino_Kit/pins_arduino.h Outdated Show resolved Hide resolved
@boramonideep
Copy link
Collaborator Author

@boramonideep @jaenrig-ifx All your fixes are BACK TO FRONT

To follow the Hardware and Software Arduino pin referenceing WHATEVER the ID says Pin 2 and 3 MUST be interrupts NOT arduino pin 26.

This means om ANY board with the same connectors especially the XMC boards the same shield and SOFTWARE will work the same WITHOUT MODIFICATION !!

By having interrupt on Arduino pin 26 you have BROKEN the Arduino Uno Shieled R3 compatability. The idents are the LEAST of your worries. The pin order in software to match order on connectors matter regardless of ident.

You change the definition of pin 2 and 3 to match what SHOULD happen NOT the ident or amy other weird method.

In Arduino NOBODY use P1.4 referencing, they use Arduino pin wiring definitions

So the ident is wrong, you obviously understand little about the arduino structure

I also agree with @djaumann ) about putting comments about AD_AUX1 to AD_AUX_n

Also as commented above you need to instigate a board agnostic method of reset pin handling as in

#define HAS_GPIO_RESET #define GPIO_RESET_PIN

This is a minimalistic approach to adding support, as in it works if you stand on one leg with your arm behind your back singing "Yankee doodle dandy"

Thanks for the comment. Since now P0.5 appears at the Pin 2 location, it was tried to route INT0 through it but in vain. Hence, pin 3 (P1.1) had to named INT0 and pin 26 (P1.4) as INT1. We understand that this is not compatible with the Arduino Uno Rev3 Form Factor, but this was the only way forward.

@boramonideep boramonideep merged commit 72525a7 into Infineon:develop Jul 4, 2022
@techpaul
Copy link
Contributor

techpaul commented Jul 4, 2022

@boramonideep @jaenrig-ifx

I see you have merged this into develop I do hope someone gets time to tidy up this mess and laziness. I was thing of using the board on a project as obviously the hardware and documentation were released before proper checks and this has now been rushed through I will have to use different micros/board on a paying project.

I do not have time at present to fix your mess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants