Debug build flag + change to sleep behavior in debug mode #4097

bulislaw · 2017-03-31T15:24:11Z

Description

Introduce new macro MBED_DEBUG that's only defined for debug profile. Right now NDEBUG is defined only for release profile, which makes it impossible to separate develop and debug builds features.
Profile used by online IDE is develop, we probably don't want disable sleep for profile which people actually may use in 'production'. This PR makes sleep a noop only for the debug builds.

Status

READY

CC: @theotherjimmy @0xc0170

bulislaw · 2017-03-31T15:24:23Z

/morph test-nightly

mbed-bot · 2017-03-31T16:11:58Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1809

Build failed!

theotherjimmy

No tools changes, so they look fine.

bridadan · 2017-04-03T19:22:54Z

FYI @bulislaw Looks like there's a build failure on this one

bulislaw · 2017-04-04T08:51:57Z

One of the board was still declaring sleep rather than hal_sleep, fixed now.

/morph test-nightly

bulislaw · 2017-04-04T09:54:00Z

retest uvisor

0xc0170

+1

we might want to document this also in the mbed_sleep.h - why NDEBUG is not the way to go as mentioned in the first description here (develop has been the default profile). To answer any questions NDEBUG vs MBED_DEBUG?

bulislaw · 2017-04-04T10:17:18Z

I think we should document it, but it should be in some generic place, where we document macros, rather than for sleep specific. I don't think we have anything close to that.

mazimkhan · 2017-04-04T10:18:30Z

@Patater can you please check. All uvisor enabled profiles fail tests.

mbed-bot · 2017-04-04T11:39:39Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1823

Build failed!

0xc0170 · 2017-04-04T12:03:43Z

Still maxim failure for sleep:

06:32:38         [DEBUG] Link: /home/jenkins/bin/ARM_Compiler_5.06u3/bin/armlink --via /home/jenkins/mbed_jenkins/workspace/bm_wrap/1912/mbed-os/BUILD/tests/MAX32630FTHR/ARM/./TESTS/mbed_drivers/ticker/.link_files.txt
06:32:38         [DEBUG] Return: 1
06:32:38         [DEBUG] Errors: Error: L6218E: Undefined symbol sleep (referred from sleep.o).
06:32:38         [DEBUG] Errors: Finished: 0 information, 0 warning and 1 error messages.

bulislaw · 2017-04-04T12:41:54Z

/morph test-nightly

mbed-bot · 2017-04-04T16:22:08Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1824

All builds and test passed!

Patater · 2017-04-04T17:12:36Z

I used git bisect on the commits in this PR, manually clearing all the pyc files between builds as otherwise the uVisor tests would still fail after going back to older commits.

Indeed commit the commit "Disable sleep and deepsleep when MBED_DEBUG macro is defined" causes the uVisor release-mode tests to fail. I'm not sure why, as it doesn't appear like that commit should have any effect on the uVisor tests passing or not, but it does.

Maybe the TARGET_DEBUG and TARGET_RELEASE feature is not working any longer after the addition of something after -g in the debug profile? Although if that were the case, I'd expect the uVisor tests to fail after the "Introduce MBED_DEBUG macro for debug build profile" commit.

bulislaw · 2017-04-05T09:08:29Z

It's weird because sleep behavior is not changed for 'release' or 'debug' profiles, only the 'develop' profile changes.

Patater · 2017-04-05T09:51:51Z

I debugged a bit further. Previously (before "Disable sleep and deepsleep when MBED_DEBUG macro is defined"), in uVisor-enabled release builds, the rtos idle thread never tried entering sleep Now, an attempt to enter sleep is made in uVisor-enabled release builds, but how sleep is being entered is faulting. The fault is due to attempting to read and write to the SCR register (in SMC_SetPowerModeVlpw) from unprivileged code.

In uVisor-enabled debug builds, no attempt to enter sleep is made, so hal_sleep isn't called and no failure happens.

bulislaw · 2017-04-05T09:56:27Z

Ah right so you use mbed OS develop profile and uVisor release build, in this case yes the behaviour changed. But that means the bug was there, it was just hidden by the sleep not being there for default profile. I've changed it for two reasons, one is that many people use online ide therefore their boards wouldn't sleep at all, second, I was afraid we will hide bugs as most people will use and deploy develop profile. I'm guessing that's exactly what we hit, do you know what's going wrong with the sleep?

Patater · 2017-04-05T09:59:28Z

Where can I read more about this "develop" and "default" profiles? I thought we only have "release" and "debug". Maybe this is another dimension.

Yes, this bug was present in the past; your PR doesn't introduce it, only expose it. We never ran our tests with a version of mbed OS that actually attempted to sleep.

bulislaw · 2017-04-05T10:01:35Z

Well some high level info here https://github.com/bulislaw/Handbook/blob/1d7783db92c6a514912ba0bfe841db40852d2dcd/docs/dev_tools/build_profiles.md and actual differences here https://github.com/ARMmbed/mbed-os/tree/master/tools/profiles

Patater · 2017-04-05T10:05:38Z

The default profile is "develop" with the CLI tools. I had assumed it was "release". So, yes, I am running the "develop" profile and mistakenly calling it the "release" profile.

What tests does mbed OS have to ensure that sleep is being entered appropriately? I'll run blinky with a debugger attached and see if we ever call hal_sleep before and after your PR with both the "release" and "develop" profiles.

Could your PR update https://github.com/ARMmbed/mbed-os/tree/master/tools/profiles to mention that develop now enters sleep?

bridadan · 2017-04-07T15:24:48Z

@bulislaw could you resolve the conflict?

0xc0170 · 2017-04-10T09:09:35Z

@bulislaw Can you please rebase (remove the merge commit) ?

bulislaw · 2017-04-10T10:41:17Z

@0xc0170 done

0xc0170 · 2017-04-10T11:13:23Z

/morph test-nightly

bridadan · 2017-04-10T14:17:27Z

/morph test-nightly

mbed-bot · 2017-04-10T16:41:22Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1867

Example Build failed!

bulislaw · 2017-04-11T10:07:53Z

@bridadan that failure looks like CI issue, maybe it run out of license or something like that? no much info

bridadan · 2017-04-11T14:44:40Z

@bulislaw I've seen this issue crop up a few times and it's always on IAR, but I have trouble reproducing it. I'll punch it one more time.

/morph test-nightly

mbed-bot · 2017-04-12T04:06:11Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1885

Test failed!

0xc0170 · 2017-04-12T07:45:53Z

Different error (not related I believe for this patch)

/morph test-nightly

mbed-bot · 2017-04-12T11:17:32Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1890

All builds and test passed!

theotherjimmy · 2017-04-18T20:04:57Z

This contains a fix for a bug that is blocking export builds. Merging.

sg- · 2017-04-19T06:35:50Z

which makes it impossible to separate develop and debug builds features.
I'm not totally sold on this one.

The reason for debug was to disable optimizations and enable single step debug builds.
The reason for release was to shrink the code.
The reason for develop was to maintain the legacy settings

Given this takes a new macro and uVisor exclusion to resolve it seems fishy. What problem is this actually solving?

This contains a fix for a bug that is blocking export builds. Merging.

This is not a good reason to merge. That change is a fix unrelated to this PR therefore should not have been part of this PR. It would have been better sent as its own PR

sg- · 2017-04-19T06:53:46Z

Going even further back 6a045a4 we seem to have added NDEBUG where version supported to work didnt make any efforts for release builds. https://github.com/ARMmbed/mbed-os/tree/db49c1edebc3516ee61197641f7cfbdeaea5482f

I'm thinking this should be reverted and NDEBUG removed if it fixes an issue we broke between mbed OS 5 and mbed OS 2. Unless there is another reason for this but it all seems like something we may want, haven't documented therefore is very confusing.

bulislaw · 2017-04-19T08:24:19Z

Main reason for this PR is confusing behavior, the original change was assuming that people actually will user release builds for 'release'. As it seems not only they can't while using online compiler, but usually don't when they use CLI. We got couple of people asking about the sleep not working and I agree it should be on in the default profile. At the same time it feels like sleep it should be disabled in debug mode, not to mention it breaks local file system (if we care about it).

bulislaw added needs: CI needs: review labels Mar 31, 2017

bulislaw self-assigned this Mar 31, 2017

This was referenced Mar 31, 2017

Build profile docs change related to sleep ARMmbed/mbed-os-5-docs#140

Merged

EFM32: Sleep function does not cause the MCU to go to a sleep mode #4082

Closed

theotherjimmy approved these changes Mar 31, 2017

View reviewed changes

bridadan added needs: work and removed needs: CI labels Apr 3, 2017

0xc0170 approved these changes Apr 4, 2017

View reviewed changes

0xc0170 removed the needs: work label Apr 4, 2017

bulislaw force-pushed the build_debug_macro branch from cefba0b to f0a79e6 Compare April 4, 2017 12:40

bridadan added the needs: CI label Apr 4, 2017

bulislaw added 4 commits April 10, 2017 11:40

Introduce MBED_DEBUG macro for debug build profile

cc71f3f

Disable sleep and deepsleep when MBED_DEBUG macro is defined

57dc2a5

MAX32630: Fix sleep definitions

4cd6068

Disable sleep when uVisor is in use

c5f0ad5

bulislaw force-pushed the build_debug_macro branch from b96cbbc to c5f0ad5 Compare April 10, 2017 10:40

0xc0170 removed the needs: review label Apr 10, 2017

0xc0170 added ready for merge and removed needs: CI labels Apr 12, 2017

bulislaw mentioned this pull request Apr 18, 2017

Deepsleep and sleep does not work in STM32L0 series #4191

Closed

theotherjimmy merged commit fb8fda3 into ARMmbed:master Apr 18, 2017

theotherjimmy added release-version: 5.5.0-rc1 and removed ready for merge labels Apr 18, 2017

sg- mentioned this pull request Apr 19, 2017

Revert "Debug build flag + change to sleep behavior in debug mode" #4200

Closed

bridadan mentioned this pull request Apr 21, 2017

Release candidate for mbed-os-5.4.4 #4209

Merged

NeilMacMullen mentioned this pull request Jul 6, 2017

UBLOX EVK NINA B1: mbed 5.5 regression on Serial class (nrf5) #4686

Closed

Debug build flag + change to sleep behavior in debug mode #4097

Debug build flag + change to sleep behavior in debug mode #4097

Conversation

bulislaw commented Mar 31, 2017

Description

Status

bulislaw commented Mar 31, 2017

mbed-bot commented Mar 31, 2017

Result: FAILURE

Output

theotherjimmy left a comment

Choose a reason for hiding this comment

bridadan commented Apr 3, 2017

bulislaw commented Apr 4, 2017

bulislaw commented Apr 4, 2017

0xc0170 left a comment

Choose a reason for hiding this comment

bulislaw commented Apr 4, 2017

mazimkhan commented Apr 4, 2017

mbed-bot commented Apr 4, 2017

Result: FAILURE

Output

0xc0170 commented Apr 4, 2017

bulislaw commented Apr 4, 2017

mbed-bot commented Apr 4, 2017

Result: SUCCESS

Output

Patater commented Apr 4, 2017

bulislaw commented Apr 5, 2017

Patater commented Apr 5, 2017 • edited Loading

bulislaw commented Apr 5, 2017

Patater commented Apr 5, 2017 • edited Loading

bulislaw commented Apr 5, 2017

Patater commented Apr 5, 2017 • edited Loading

bridadan commented Apr 7, 2017

0xc0170 commented Apr 10, 2017

bulislaw commented Apr 10, 2017

0xc0170 commented Apr 10, 2017

bridadan commented Apr 10, 2017

mbed-bot commented Apr 10, 2017

Result: FAILURE

Output

bulislaw commented Apr 11, 2017

bridadan commented Apr 11, 2017

mbed-bot commented Apr 12, 2017

Result: FAILURE

Output

0xc0170 commented Apr 12, 2017

mbed-bot commented Apr 12, 2017

Result: SUCCESS

Output

theotherjimmy commented Apr 18, 2017

sg- commented Apr 19, 2017 • edited Loading

sg- commented Apr 19, 2017

bulislaw commented Apr 19, 2017

Patater commented Apr 5, 2017 •

edited

Loading

Patater commented Apr 5, 2017 •

edited

Loading

Patater commented Apr 5, 2017 •

edited

Loading

sg- commented Apr 19, 2017 •

edited

Loading