Skip to content

Use mbed_error_printf instead of debug for ISR friendly prints #9260

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
Merged
merged 1 commit into from
Jan 10, 2019
Merged

Use mbed_error_printf instead of debug for ISR friendly prints #9260

merged 1 commit into from
Jan 10, 2019

Conversation

deepikabhavnani
Copy link

@deepikabhavnani deepikabhavnani commented Jan 4, 2019 •
edited
Loading

Description

Resolves: #9216

All the debug calls in sleep manager for sleep tracing should be changed to mbed_error_printf since most sleep API's and calls are interrupt and thread safe. Fixed here.

Issue was seen only with debug profile and was not captured in develop profile because MBED_TRAP_ERRORS_ENABLED flag was not set in develop profile. PR #9277

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@kjbracey-arm

@deepikabhavnani
Copy link
Author

@ARMmbed/mbed-os-maintainers

Enabling -DMBED_TRAP_ERRORS_ENABLED=1 in develop profile will be considered as breaking change?

@deepikabhavnani deepikabhavnani mentioned this pull request Jan 4, 2019
Closed
@ciarmcom ciarmcom requested review from bulislaw, kjbracey, theotherjimmy and a team January 5, 2019 00:00
@ciarmcom
Copy link
Member

ciarmcom commented Jan 5, 2019

@deepikabhavnani, thank you for your changes.
@theotherjimmy @bulislaw @kjbracey-arm @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-maintainers please review.

kjbracey
kjbracey previously requested changes Jan 7, 2019
View reviewed changes
@0xc0170 0xc0170 changed the title Flags fix Sleep: error flags fix Jan 7, 2019
@deepikabhavnani deepikabhavnani mentioned this pull request Jan 7, 2019
Merged
Use mbed_error_printf instead of debug for ISR friendly prints
7e5e536 
Internally in sleep tracing `debug` was used for trace prints, but
all sleep API's are ISR safe and used in interrupts. This resulted in
hardfaults / errors.

Solution is to use `mbed_error_printf` instead for printing on UART
@deepikabhavnani deepikabhavnani changed the title Sleep: error flags fix Use mbed_error_printf instead of debug for ISR friendly prints Jan 7, 2019
@deepikabhavnani deepikabhavnani dismissed kjbracey’s stale review January 7, 2019 16:10

PR is split and review comments addressed. Please review again

@deepikabhavnani deepikabhavnani removed the request for review from a team January 7, 2019 16:14
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 9, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 9, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@cmonr cmonr merged commit 9e48f75 into ARMmbed:master Jan 10, 2019
@deepikabhavnani deepikabhavnani deleted the flags_fix branch January 10, 2019 16:51
@TacoGrandeTX
Copy link
Contributor

@deepikabhavnani @ARMmbed/mbed-os-maintainers Is there a better solution for this? Having to build for the debug profile (which disallows sleep) seems counter-intuitive when using the sleep tracer to diagnose sleep issues. I just hit the breaking change in the developer profile (#9260 (comment)) when trying to validate #9685.

@TacoGrandeTX
Copy link
Contributor

After uncovering a problem with PR #9685 - I think my comment above can be ignored... I thought one now had to use the debug profile to see LOCK/UNLOCK messages from the sleep tracer but I see that's not the case. @deepikabhavnani could you confirm?

@deepikabhavnani
Copy link
Author

deepikabhavnani commented Feb 12, 2019 •
edited
Loading

@TacoGrandeTX - Yes, the goal here was not to move to debug profile, rather enable prints in debug and develop profile from ISR context (sleep/deepsleep)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@kjbracey kjbracey kjbracey approved these changes

Assignees
No one assigned
Labels
release-version: 5.11.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@deepikabhavnani @ciarmcom @0xc0170 @mbed-ci @TacoGrandeTX @kjbracey @cmonr