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

SOFT 298: delay_us implemented without soft timer #364

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

selinahsu
Copy link
Contributor

No description provided.

@ShiCheng-Lu
Copy link
Contributor

I could change x86 implementation to be more similar to STM32, if that is desired?

@ShiCheng-Lu
Copy link
Contributor

Now, do I know why test_bts7040_enable_fails_during_fault failed? Absolutely not...
seems like the callback isn't being called when a fault is set

  // Make sure fault clears after time elapsed and values return to normal
  delay_ms(40);
  TEST_ASSERT_TRUE(bts7040_get_output_enabled(&s_storage));

delay is preventing fault from being cleared?
maybe it's because the old delay uses wait() so the processor can process the fault clearing,
while the new delay without soft_timer needs to be a busy loop?

…critical section (interrupts disabled), jank fix, but delay really shouldn't be used in critical section anyway
Copy link
Collaborator

@ryandancy ryandancy left a comment

Choose a reason for hiding this comment

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

No full review yet (I was trying to get it to work on the hardware I have and it's decided not to work) but one question: is there a reason we still need the soft timer solution when not in an interrupt if your other solution works?

@ryandancy
Copy link
Collaborator

Also I'm pretty sure that time.h etc should work on stm32 (unless you got a compile error in which case never mind) - the stm32 standard library we use is called newlib (source downloadable/searchable at that link) and I think all the libraries are implemented. @ShiCheng-Lu

@ShiCheng-Lu
Copy link
Contributor

What on earth? :< We've got quite a few problems here.
time.h with newlib doesn't have the functions to get micro-second precision. (only seconds)

at least on x86, the time with clock_gettime() is ever so slightly faster than times in soft_timer which uses timers that rely in interrupt. so a lot of delays become just short enough to break most tests that use soft_timer to do some action.

although it might be accurate on stm32 as rightnow it is using the same time TIM2 that soft_timer uses. But that build did fail with bts7040 test in ms-drivers. Do not know why.

I would probably suggest leaving delay_us implemented with soft timers, and forbid the use of delay_us() in callbacks. Implement another delay function with system clock that is slightly less accurate to the rest of the program if delay is ever needed in a callback.
Other option is the way I have it now, where it checks if interrupts are enabled, and uses soft_timer or sys time accordingly.

@ShiCheng-Lu ShiCheng-Lu removed their assignment Sep 18, 2021
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.

3 participants