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.

Sign up for GitHub

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

Jump to bottom

Asserts for checking that network calls are run on system thread #1146

Merged
merged 4 commits into from
Nov 29, 2016
Merged

Asserts for checking that network calls are run on system thread #1146

merged 4 commits into from
Nov 29, 2016

Conversation

avtolstoy
Copy link
Member

@avtolstoy avtolstoy commented Oct 22, 2016 •
edited by technobly
Loading

Closes #1040.

  • Particle.keepAlive() should be run on system thread
  • DEBUG_THREADING compilation option added to enable thread asserts
  • ASSERT_ON_SYSTEM_OR_MAIN_THREAD and ASSERT_ON_SYSTEM_THREAD asserts added
  • Added ASSERT_ON_SYSTEM_OR_MAIN_THREAD and ASSERT_ON_SYSTEM_THREAD asserts to spark_protocol_* calls

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
  • Add to CHANGELOG.md after merging (add links to docs and issues)

Internal

  • Added asserts for checking that network calls are run on system thread. [Closes #1040]

@avtolstoy avtolstoy added this to the 0.7.x milestone Oct 22, 2016
@avtolstoy avtolstoy force-pushed the feature/assert-network-calls-system-thread branch from 936d60c to 079a407 Compare October 22, 2016 16:46
m-mcgowan
m-mcgowan approved these changes Oct 29, 2016
View reviewed changes
Copy link
Contributor

@m-mcgowan m-mcgowan left a comment

Choose a reason for hiding this comment

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

looks good. I was thinking about commenting that some of the code might be wrapped in a conditional compile for DEBUG_THREAD, but it's not really worth it (and we still have to keep it in the dynalib interface)

@avtolstoy avtolstoy mentioned this pull request Nov 23, 2016
Merged
7 tasks
@technobly
Merge branch 'develop' into feature/assert-network-calls-system-thread
e7b3a61 
  Conflicts:
	communication/src/spark_protocol_functions.cpp
	system/inc/system_dynalib.h
	system/inc/system_dynalib_cloud.h
	system/src/system_task.cpp
@technobly technobly merged commit e854596 into develop Nov 29, 2016
@technobly technobly deleted the feature/assert-network-calls-system-thread branch November 29, 2016 00:14
@technobly technobly modified the milestones: 0.7.x, 0.6.1 Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m-mcgowan m-mcgowan m-mcgowan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.6.1
Development

Successfully merging this pull request may close these issues.
3 participants
@avtolstoy @m-mcgowan @technobly