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

Feature/delay event processing #1094

Merged
merged 2 commits into from Aug 27, 2016
Merged

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Aug 14, 2016

Fixes issue #1055

system_delay_ms should not use !system_thread_get_state(NULL) to determine whether to run event loop or not, as it is still possible to create a thread with SYSTEM_THREAD(DISABLED) and execute delay() in that thread, which will cause the event loop to be processed from that thread:
https://github.com/spark/firmware/compare/feature/delay-event-processing?expand=1#diff-de0a60700cd1e842d6a89faaba614c8cL446

This PR:

  • Changes system_delay_ms condition to if ((!PLATFORM_THREADING || APPLICATION_THREAD_CURRENT()) && !HAL_IsISR())
    • !system_thread_get_state(NULL) -> !PLATFORM_THREADING
  • Exposes APPLICATION_THREAD_CURRENT()/SYSTEM_THREAD_CURRENT() macros as application_thread_current/system_thread_current dynalib functions
  • Adds wiring/threading_no_threading_delay test

Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation (N/A)
  • Add to CHANGELOG.md after merging (add links to docs and issues)

BUGFIX

  • Do not run the event loop from delay() when threading is enabled. Fixes #1055

@monkbroc
Copy link
Member

Fantastic. Code looks good to me. Nice work coming up with such a detailed test.

@technobly technobly added this to the 0.6.x milestone Aug 23, 2016
@technobly technobly merged commit 426faf1 into develop Aug 27, 2016
@technobly technobly deleted the feature/delay-event-processing branch October 27, 2016 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants