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

tests/drivers/counter/counter_basic_api: instable test status on STM32 boards #25366

Closed
ABOSTM opened this issue May 15, 2020 · 2 comments · Fixed by #25367
Closed

tests/drivers/counter/counter_basic_api: instable test status on STM32 boards #25366

ABOSTM opened this issue May 15, 2020 · 2 comments · Fixed by #25367
Assignees
Labels
area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug
Milestone

Comments

@ABOSTM
Copy link
Collaborator

ABOSTM commented May 15, 2020

Warning: it is needed to cherry picking PR #25364 to be able to reach this stage.

While exectuing test tests/drivers/counter/counter_basic_api, from time to time, test fails during
step test_short_relative_alarm, with 1 less callback count than expected.

To Reproduce
Steps to reproduce the behavior:

  1. west -v build -p auto -b nucleo_f746zg tests/drivers/counter/counter_basic_api
  2. See error

Expected behavior
Test Passed

Screenshots or console output

starting test - test_short_relative_alarm
D: Set Alarm: 10

Testing RTC_0
D: Set Alarm: 20

D: Set Alarm: 23

D: Set Alarm: 26

D: Set Alarm: 29

D: Set Alarm: 32

D: Set Alarm: 34

D: Set Alarm: 37

D: Set Alarm: 40

D: Set Alarm: 43

D: Set Alarm: 46

D: Set Alarm: 49

D: Set Alarm: 52

D: Set Alarm: 55

D: Set Alarm: 58

D: Set Alarm: 60

D: Set Alarm: 63

Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/counter/counter_basic_api/src/test_counter.c:711: test_short_relative_alarm_instance: (i + 1 equal to alarm_cnt)

RTC_0: Expected 17 callbacks, got 16

@ABOSTM ABOSTM added the bug The issue is a bug, or the PR is fixing a bug label May 15, 2020
@ABOSTM
Copy link
Collaborator Author

ABOSTM commented May 15, 2020

Analysing this issue it appears to me that in driver,

  • current value of RTC is read,
  • after some computation and convertion, we add expected tick to rtc read value
  • program the alarm with it obtained value.

Knowing that RTC frequency is 1Hz, the current value of the RTC which is read, correspond to the last ended second. But in fact the next second is already running for some timer.
In other word, when programming N ticks, the expiration of the alarm will occur between (N-1) ticks and N ticks, due to the ongoing tick. It means that application requesting N Ticks may have less than expected.
In the particullar case where only 1 tick is requested, alarm will be propgrammed to next tick, and it will therorically expire beween 0 and 1 tick. Saying it differently, the RTC +1 tick event may occurs before Alarm is actually programmed (during computation/convertion/programmation). Specially if next tick is about to finish when RTC value is read.

My proposal would be to add 1 tick to alarm setting to compensate the ongoing tick. Thus have an expiration between N and N+1. Thus we guarantee to application the expected timing, and it give driver enough time to program alarm before the event occurs.
Proposal of PR implementation: #25367

Does anyone see a propoblem to change this behavior ?
@pabigot , @nordic-krch

With the proposed PR, samples/drivers/counter/alarm is now failed because of expected regex
regex:
- "Counter alarm sample"
- "Set alarm in 2 sec"
- "!!! Alarm !!!"
- "Now: 2"
But it could be change to accept "Now:3" (or even accept both [2|3])

@carlescufi carlescufi added area: Tests Issues related to a particular existing or missing test platform: STM32 ST Micro STM32 labels May 15, 2020
@erwango erwango added the priority: medium Medium impact/importance bug label May 15, 2020
@erwango erwango added this to the v2.3.0 milestone May 15, 2020
@ABOSTM
Copy link
Collaborator Author

ABOSTM commented May 15, 2020

Proposed PR for samples/drivers/counter/alarm: #25371

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug platform: STM32 ST Micro STM32 priority: medium Medium impact/importance bug
Projects
None yet
3 participants