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

Remove printf call from interrupt context #31

Merged
merged 1 commit into from
Feb 21, 2017

Conversation

LMESTM
Copy link
Contributor

@LMESTM LMESTM commented Feb 16, 2017

With ARM compiler at least, if we enable DEBUG, the InterruptIn will fail
in timeout as the program would crash calling printf in the cbfn as this
happens in Interrupt context. Let's remove it

With ARM compiler at least, if we enable DEBUG, the InterruptIn will fail
in timeout as the program would crash calling printf in the cbfn as this
happens in Interrupt context. Let's remove it
@BlackstoneEngineering
Copy link
Contributor

@mray19027 be aware of this for the work you are doing. Make sure to test with armcc and gcc and iarcc when validating.

@LMESTM what version of armcc are you using?

@@ -33,7 +33,6 @@ volatile bool result = false;
void cbfn(void)
{
result = true;
DEBUG_PRINTF("\t**** Interrupt Triggered!\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch !

@0xc0170
Copy link
Collaborator

0xc0170 commented Feb 20, 2017

LGTM

I am thinking how we can log from interrupt, use deferred events? @LMESTM Any proposals?

@0xc0170
Copy link
Collaborator

0xc0170 commented Feb 21, 2017

I'll merge this, we should not print anything from interrupt, find another approach for this (buffer serial or deferred events).

@0xc0170 0xc0170 merged commit 27a7850 into ARMmbed:master Feb 21, 2017
@BlackstoneEngineering
Copy link
Contributor

This should not have been merged without adding a replacement. The existing printf's were only in debug, now instead of having some debug there is no debug whatsoever for this. I would argue that without a replacement the tests are made worse rather than better.

@0xc0170
Copy link
Collaborator

0xc0170 commented Feb 22, 2017

I would argue that without a replacement the tests are made worse rather than better.

If the test fails because of it (in this case timeout), I think its better not to have the debug print (a bad practice to invoke printf in interrupt routines). An issue should have been created. Ill do it right-away.

I'll fix it and revert this one ;)

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.

None yet

3 participants