Skip to content

tests: drivers: adc_dma: fix sampling interval failing NXP tests #56104

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

Conversation

heinwessels
Copy link
Contributor

In 6e21ebf the sampling interval was changed, which caused the issue #56070. This is fixed by allowing different boards to specify a sampling period.

Also changed the test_task_with_interval to verify that the supplied interval was used.

@heinwessels heinwessels requested a review from nashif as a code owner March 22, 2023 12:06
@heinwessels heinwessels marked this pull request as draft March 22, 2023 12:06
@zephyrbot zephyrbot added the area: ADC Analog-to-Digital Converter (ADC) label Mar 22, 2023
@zephyrbot zephyrbot requested a review from anangl March 22, 2023 12:06
@heinwessels
Copy link
Contributor Author

@hakehuang would you mind testing this code?

@nashif nashif requested review from teburd and hakehuang March 22, 2023 12:58
@heinwessels heinwessels marked this pull request as ready for review March 22, 2023 13:05
@heinwessels
Copy link
Contributor Author

@teburd this code will fix half the issue you observed. It should finish the first two tests, and the crash on the third. The other half is caused by a few limitations in the NXP ADC driver, which @hakehuang is fixing, and will likely have a PR ready for soon.

@teburd
Copy link
Collaborator

teburd commented Mar 22, 2023

Results

*** Booting Zephyr OS build zephyr-v3.3.0-1420-g16ccf91e2e17 ***
Running TESTSUITE adc_dma
===================================================================
START - test_adc_asynchronous_call
Samples read: 0x0373 0x0000 0x007f 0x0000 0x0373 0x0000 
    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/adc/adc_dma/src/test_adc.c:182: check_samples: (INVALID_ADC_VALUE not equal to sam)
[5] should be empty
 FAIL - test_adc_asynchronous_call in 0.021 seconds
===================================================================
START - test_adc_invalid_request
E: Invalid resolution
E: Invalid resolution
Samples read: 0x0309 0x0000 
    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/adc/adc_dma/src/test_adc.c:182: check_samples: (INVALID_ADC_VALUE not equal to sam)
[1] should be empty
 FAIL - test_adc_invalid_request in 0.022 seconds
===================================================================
START - test_adc_repeated_samplings
repeated_samplings_callback: done 1
Samples read: 0x030b 0x0000 
    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/adc/adc_dma/src/test_adc.c:182: check_samples: (INVALID_ADC_VALUE not equal to sam)
[1] should be empty
ASSERTION FAIL [0] @ WEST_TOPDIR/zephyr/kernel/sched.c:1750
        aborting essential thread 0x200007e0
E: r0/a1:  0x00000004  r1/a2:  0x000006d6  r2/a3:  0x00000080
E: r3/a4:  0x00002e09 r12/ip:  0x00000000 r14/lr:  0x00006663
E:  xpsr:  0x41000011
E: Faulting instruction address (r15/pc): 0x00007c44
E: >>> ZEPHYR FATAL ERROR 4: Kernel panic on CPU 0
E: Fault during interrupt handling

E: Current thread: 0x200007e0 (main)
E: Halting system

@heinwessels
Copy link
Contributor Author

Results

Ah, that looks correct. The crash and failing tests will be fixed by hakehuang.

In 6e21ebf the sampling interval was changed, which caused the
issue zephyrproject-rtos#56070. This is fixed by allowing different boards to
specify a sampling period.

Also changed the test_task_with_interval to verify that
the supplied interval was used.

Signed-off-by: Hein Wessels <[email protected]>
@heinwessels heinwessels force-pushed the test-adc-dma-interval-fix branch from 16ccf91 to f4970da Compare March 22, 2023 14:13
@hakehuang
Copy link
Collaborator

hakehuang commented Mar 22, 2023

Ah, that looks correct. The crash and failing tests will be fixed by hakehuang.

@heinwessels , one qeustion, how can I create a PR base on your PR?

@heinwessels
Copy link
Contributor Author

Ah, that looks correct. The crash and failing tests will be fixed by hakehuang.

@heinwessels , one qeustion, how can I create a PR base on your PR?

Can we merge this PR as is? @anangl @teburd It doesn't block CI, as it's not run. Maybe @hakehuang can show that all tests pass if he applies his changes on top of mine.

Other option is that you cherry-pick my commit into a new branch, and then apply your changes on top of that. However, I would prefer the first option. You could even make a commit fixing it on your own branch that's public somewhere, and I cherry-pick it into this PR.

@hakehuang
Copy link
Collaborator

@hakehuang would you mind testing this code?

with your fix and my PR #56120 test log is patch there

Copy link
Collaborator

@hakehuang hakehuang left a comment

Choose a reason for hiding this comment

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

need work with
#56120

@fabiobaltieri fabiobaltieri added the Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc. label Mar 23, 2023
@fabiobaltieri fabiobaltieri merged commit 5ec8aa4 into zephyrproject-rtos:main Mar 23, 2023
@fabiobaltieri
Copy link
Member

@anangl sorry did not realize that the test was not NXP specific and merged without waiting for your approval, lmk if this was not ok and should be reworked.

@heinwessels heinwessels deleted the test-adc-dma-interval-fix branch March 27, 2023 09:24
carlescufi pushed a commit that referenced this pull request Apr 13, 2023
add support for async call and repeat sample test

1. change the DMA req to 2 byte each
2. increase the buffer pre-dma
3. add protection on invalid buffer

depends on: PR #56104
fixing: issue #56070

Signed-off-by: Hake Huang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) Hotfix Fix for issues blocking development, i.e. CI issues, tests failing in CI, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants