Skip to content

changed Makefile.test, updated submodule for arduino-core-tests #336

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 5 commits into from
Apr 16, 2025

Conversation

Frederikwag
Copy link
Contributor

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
The submodule "Arduino-Core-Test" in the repo "XMC-for-Arduino" linked to an older version. The version of "Arduino-Core-Test" was updated to the newest one, the different tests were executed and it was documented which tests are also working. The files where adapted accordingly.

Related Issue
It's not an issue, but tasks that still need to be done:

  1. Interrupts: It exists an open ticket for the interrupt-problem. A sending interrupt is not received by an receiving pin. The existing test shows that 4/6 tests don't pass, which CAN be traced back to this. Therefore, the interrupt-functionality needs to be solved first. BUG-Ticket needs to be created!
  2. Timing: the following test function is not passed: RUN_TEST_CASE(time_single_internal, testDelayMicroseconds)
    --> Expected Delay: 500000 microseconds; Actual Delay: 506483 microseconds; Tolerance: 100 microseconds (result: values not within delta 100)
  3. IIC is not working for 1 board only: make FQBN=Infineon:xmc:XMC4700_Relax_Kit PORT=COM28 UNITY_PATH=\Unity test_wire_connected1_pingpong monitor --> There is an open ToDo

Context
Quiet similar to previous push (added-spi), but tried to reduce amount of changes. Therefore checked code again.

@ederjc ederjc self-requested a review April 8, 2025 15:44
Copy link
Member

@ederjc ederjc left a comment

Choose a reason for hiding this comment

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

Hi @Frederikwag, your commit message check fails. Please have a look here and then here.

Please let me know if you have questions.

Regarding the failing HIL checks we can see what's wrong once the commit checks are succeeding.

@jaenrig-ifx
Copy link
Member

The fqbn vendor renaming and the ports are specific to @Frederikwag local setup.
@LinjingZhang what is the idea behind this test makefile?
I see it as a preliminary list of the tests that has been checked so far (and will be run in the HIL). Is that it?
If we want (we should if it is not temporary) enable any user to run the list of tests locally, we should provide a way to pass the hardcoded value as parameters.

@LinjingZhang
Copy link
Collaborator

The fqbn vendor renaming and the ports are specific to @Frederikwag local setup. @LinjingZhang what is the idea behind this test makefile? I see it as a preliminary list of the tests that has been checked so far (and will be run in the HIL). Is that it? If we want (we should if it is not temporary) enable any user to run the list of tests locally, we should provide a way to pass the hardcoded value as parameters.

Yes, it is. This is just a test command that we have validated and will be used in HIL in the future.
We can also add it to .gitignore, but for now I think it's a good reference for developers.

Copy link
Collaborator

@LinjingZhang LinjingZhang left a comment

Choose a reason for hiding this comment

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

Please revise based on the review comments.
I remember we also had some test failures (invalid pins) when running gpio, not found in the description

# 1 boards

make FQBN=arduino-git:xmc:XMC4700_Relax_Kit PORT=/dev/ttyACM1 UNITY_PATH=\Unity test_digitalio_single monitor
Copy link
Collaborator

Choose a reason for hiding this comment

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

for gpio, we do need a test_config.h for pin definition right? Should it be added to the PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

correct added. good 👍

Copy link
Collaborator

@LinjingZhang LinjingZhang left a comment

Choose a reason for hiding this comment

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

You can ask Julian to help with commit msg ;)

_build/

echo "libraries/SPI/src/SPI.cpp" >> .gitignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

why adding echo command in .gitignore?

Copy link
Contributor Author

@Frederikwag Frederikwag Apr 15, 2025

Choose a reason for hiding this comment

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

Because everything what is related to SPI is a different branch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please check: https://www.geeksforgeeks.org/what-is-git-ignore-and-how-to-use-it/
and: https://www.geeksforgeeks.org/echo-command-in-linux-with-examples/

to understand real reason why you want to use echo and add the path in .gitignore and why I ask you to remove this code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, this time the submodule points to a unknow hash tag... please double check

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice.

Copy link
Collaborator

@LinjingZhang LinjingZhang Apr 15, 2025

Choose a reason for hiding this comment

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

I forgot to mention that it would be great to add the name of the board used here. Because xmc has a lot of different targets

@LinjingZhang LinjingZhang requested a review from ederjc April 15, 2025 12:40
@LinjingZhang LinjingZhang merged commit c7cc923 into master Apr 16, 2025
8 of 10 checks passed
@LinjingZhang LinjingZhang deleted the update_test branch April 16, 2025 13:53
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.

4 participants