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

Bring back SPIF module-specific debug logs #11163

Merged

Conversation

michalpasztamobica
Copy link
Contributor

Description

The logs are switched off by default and can be enabled with a module-specific compile switch in mbed_app.json. This way, setting the debug level will not imply that SPIF-specific logs flood the output. Instead, when SPIF developers want to debug their module, they can enable them easily.

Brought back from PR #10501

As @teetak01 suggested, I followed the example of ESP8266 and its ATParser.

Pull request type

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

Reviewers

@AriParkkila
@teetak01
@davidsaada
@bulislaw
@SeppoTakalo

@michalpasztamobica michalpasztamobica force-pushed the spif_driver_conditional_debug branch from dbd47b6 to e96d782 Compare August 5, 2019 08:34
@michalpasztamobica
Copy link
Contributor Author

Astyle fixes added.

@ciarmcom
Copy link
Member

ciarmcom commented Aug 5, 2019

@michalpasztamobica, thank you for your changes.
@teetak01 @bulislaw @AriParkkila @SeppoTakalo @davidsaada @ARMmbed/mbed-os-storage @ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@VeijoPesonen VeijoPesonen 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 to me.

*/
#ifndef MBED_CONF_SPIF_BLOCK_DEBUG
#define MBED_CONF_SPIF_BLOCK_DEBUG false
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? Is the debug_if() macro sensitive of macros being not defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, debug_if is a function. Code won't compile if MBED_CONF_SPIF_BLOCK_DEBUG is not defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should not be neccessary?

Just change the mbed_lib.json to only accept values 1 and 0.
For. ex. "options": [0, 1].

Just to clarify for general information:
Setting value 0 in mbed_lib.json defines the MACRO with value of 0.
Setting value null in mbed_lib.json does not define any MACRO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @teetak01 , you are right. I amended the code.

@mbed-ci
Copy link

mbed-ci commented Aug 5, 2019

Test run: SUCCESS

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

@michalpasztamobica michalpasztamobica force-pushed the spif_driver_conditional_debug branch from e96d782 to a5bbf2d Compare August 6, 2019 07:14
The logs are switched off by default and can be enabled with a
module-specific compile switch in mbed_app.json.

Logs brought back from PR ARMmbed#10501
@michalpasztamobica michalpasztamobica force-pushed the spif_driver_conditional_debug branch from a5bbf2d to 1fe59b7 Compare August 6, 2019 07:55
@mbed-ci
Copy link

mbed-ci commented Aug 6, 2019

Test run: SUCCESS

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

@SeppoTakalo SeppoTakalo merged commit 431c4c1 into ARMmbed:master Aug 6, 2019
@michalpasztamobica michalpasztamobica deleted the spif_driver_conditional_debug branch August 6, 2019 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants