Add mbed-os version macros #3096

infinnovation-dev · 2016-10-20T21:14:45Z

Description

Allow compile-time tests on the version of mbed-os to cope with e.g. API changes across versions.
As suggested in #3091

Status

READY

Migrations

NO

theotherjimmy · 2016-10-21T19:07:35Z

mbed.h

@@ -18,6 +18,14 @@

 #define MBED_LIBRARY_VERSION 123

+#define MBED_MAJOR_VERSION 5
+#define MBED_MINOR_VERSION 2
+#define MBED_MICRO_VERSION 0


Patch version maybe?

@infinnovation Please use patch as asked above

So I infer you want "patch" instead of "micro" (naming which I had lifted from glib). A few extra words would have saved us all some time!

0xc0170 · 2016-10-24T16:01:21Z

mbed.h

@@ -21,10 +21,11 @@
 #define MBED_MAJOR_VERSION 5
 #define MBED_MINOR_VERSION 2
 #define MBED_MICRO_VERSION 0


please remove the micro

0xc0170 · 2016-10-25T08:16:10Z

/morph test

0xc0170 · 2016-10-25T08:17:10Z

@infinnovation Thanks ! Looks good to me, I'll squash merge this PR as it should be as one commit (or you can do it)

Allow compile-time tests on the version of mbed-os to cope with e.g. API changes across versions.

0xc0170 · 2016-10-25T12:45:22Z

/morph test

mbed-bot · 2016-10-25T17:18:51Z

Result: FAILURE

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

/morph test

bridadan · 2016-10-25T17:27:23Z

The failure was from before the rebase, @0xc0170's second run should go through OK.

mbed-bot · 2016-10-25T20:40:24Z

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 939

All builds and test passed!

0xc0170 · 2016-10-26T09:52:54Z

These macros are available for also mbed 2, that should not be? If it's the case, how to protect those for only mbed 5 builds?

#if defined(MBED_CONF_RTOS_PRESENT)
// mbed 5 build
// version macros here
#endif

Should mbed lib revision macro be accessible in the mbed 5? I would say it stays, a question is then about this new mbed OS version macro. Ideas?

@c1728p9 @sg- @pan- @geky

pan- · 2016-10-26T10:25:55Z

Should mbed lib revision macro be accessible in the mbed 5?

If we treat the mbed library like any other library then it would make a lot of sense to keep the mbed lib revision macro accessible in mbed OS 5.

A library based on mbed SDK should continue to work on mbed OS. If this library defines a polyfil when it's compiled with MBED_LIBRARY_VERSION < XYZ then this should still work when it is build on mbed OS 5 and user should not add new test or modify its code because mbed OS X.Y.Z use (and not is!) mbed lib version XXX.

It is the same case with other libraries bundled within mbed OS, their revision number does not disappear once they are packaged in mbed OS.
mbed TLS X.Y.Z remains mbed TLS X.Y.Z and does not become mbed TLS undefined version in mbed OS X.Y.Z.

For the MBED_VERSION macro, what is the value of defining 2.Y.Z version when the only thing contained by 2.Y.Z versions is mbed lib ? Should it be defined only for mbed OS 5 and forwards ?

0xc0170 · 2016-10-28T09:50:54Z

For the MBED_VERSION macro, what is the value of defining 2.Y.Z version when the only thing contained by 2.Y.Z versions is mbed lib ? Should it be defined only for mbed OS 5 and forwards ?

I would propose this:

#if defined(MBED_CONF_RTOS_PRESENT)
# we got RTOS present, this is valid only for mbed 5
#define MBED_MAJOR_VERSION 5
#define MBED_MINOR_VERSION 2
#define MBED_PATCH_VERSION 0

#else
// mbed 2
#define MBED_MAJOR_VERSION 2
#define MBED_MINOR_VERSION 0
#define MBED_PATCH_VERSION MBED_LIBRARY_VERSION
#endif

0xc0170 · 2016-10-31T15:12:07Z

@c1728p9 @sg- @geky - please review above proposal if it makes sense or there's something better we could possibly do?

geky · 2016-10-31T15:35:49Z

This looks reasonable to me 👍

0xc0170 · 2016-11-01T13:05:30Z

@infinnovation What do you think? Shall we update this PR according to the proposal #3096 (comment) ?

infinnovation-dev · 2016-11-01T15:50:37Z

@0xc0170 I'm happy to go along with whatever you folks at ARM Towers decide. I ticked the "Allow edits by maintainers" option, so feel free to edit, respell, refactor, squash, merge or whatever, if that is simpler.

0xc0170 · 2016-11-02T15:24:03Z

@infinnovation Thanks, I'll pull this locally and do the changes

infinnovation-dev · 2016-11-02T20:31:00Z

One more thought: could these macros be split off into a separate .h file (mbed_version.h perhaps) with appropriate guards so that they can be used by C code as well as C++ ?

geky · 2016-11-02T20:59:56Z

One more thought: could these macros be split off into a separate .h file (mbed_version.h perhaps) with appropriate guards so that they can be used by C code as well as C++ ?

An alternative idea would be to allow mbed.h to be included from C code. We could expose only the C-compatible interfaces, and this may be better aligned with our "single entry point".

To distinguish between those 2 versions, we use MBED_CONF_RTOS_PRESENT macro. Note: mbed OS 2 versioning is 2.0.MBED_LIBRARY_VERSION

0xc0170 · 2016-11-03T14:38:14Z

I added the proposal in the comment above as a new commit.

Please review again.

One more thought: could these macros be split off into a separate .h file (mbed_version.h perhaps) with appropriate guards so that they can be used by C code as well as C++ ?

What's the use-case? I would assume that any app code is C++, and these macros would be used there?

infinnovation-dev · 2016-11-03T21:25:12Z

Without having a specific example in mind, I was thinking more of libraries: third-party drivers, middleware etc.

sg- · 2016-11-03T23:12:16Z

We don't commit or version any C interfaces. That may change and the C accessibility can if/when that is the case.

sg- · 2016-11-03T23:13:52Z

/morph test

sg- · 2016-11-03T23:19:03Z

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

mbed-bot · 2016-11-03T23:20:24Z

[Build ${MBED_BUILD_ID}]
FAILURE: Something went wrong when building and testing.

sg- · 2016-11-03T23:21:28Z

@bridadan its gotta be the bot and not my command... I'm going crazy here 🎱

bridadan · 2016-11-03T23:56:20Z

@mbed-bot: TEST

HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

bridadan · 2016-11-04T00:00:00Z

The reason the mbed 2 bot was failing was because it was always merging it with master, not with the branch that the PR was submitted against. The morph bot does correctly but the second bot didn't. So good catch there.

However, that then brings the question should this be submitted against master instead of the release branch?

sg- · 2016-11-04T01:33:00Z

However, that then brings the question should this be submitted against master instead of the release branch?

Always! everything gets submitted against master

mbed-bot · 2016-11-04T02:39:53Z

[Build 1084]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

bridadan · 2016-11-04T13:58:10Z

Always! everything gets submitted against master

In that case, @infinnovation, can you please resubmit this against master?

mbed-bot · 2016-11-04T14:05:44Z

Result: FAILURE

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

/morph test

Output

mbed Build Number: 994

Test Prep failed!

bridadan · 2016-11-04T14:19:16Z

Failure was my fault, a few machines were stuck in maintenance mode and halted the whole process. We can rerun the tests when this is submitted against master.

infinnovation-dev · 2016-11-04T22:02:12Z

Hmm, not sure my git-fu is up to this. I cherry-picked, squashed then pushed -f, but github still thinks this is a PR against mbed-os-5.2. May be simpler to create a fresh PR.

bridadan · 2016-11-04T22:06:48Z

@infinnovation Oh yes sorry for being unclear, you'll definitely need to close this and resubmit it against master as a new PR!

geky · 2016-11-04T22:37:10Z

Just wanted to see if changed the destination would work, will probably be easier to continue with #3207, which looks great

bridadan · 2016-11-04T22:45:55Z

@infinnovation Thanks for submitting the new PR! I'll go ahead and close this and we'll continue the conversation on #3207

theotherjimmy reviewed Oct 21, 2016

View reviewed changes

0xc0170 reviewed Oct 24, 2016

View reviewed changes

0xc0170 added the needs: review label Oct 24, 2016

0xc0170 added needs: CI and removed needs: review labels Oct 25, 2016

Add mbed-os version macros

5cbc65b

Allow compile-time tests on the version of mbed-os to cope with e.g. API changes across versions.

infinnovation-dev force-pushed the mbed-version branch from c4de555 to 5cbc65b Compare October 25, 2016 08:54

0xc0170 added release-version: 5.2.1 and removed needs: CI labels Oct 26, 2016

0xc0170 removed the release-version: 5.2.1 label Oct 27, 2016

This was referenced Oct 27, 2016

TARGET_NUMAKER_PFM_NUC472 dependency on mbed TLS #3006

Closed

build information as compile time defines #2004

Closed

0xc0170 added the needs: review label Oct 31, 2016

version macros - define it for mbed OS 5 and mbed OS 2

935d0f8

To distinguish between those 2 versions, we use MBED_CONF_RTOS_PRESENT macro. Note: mbed OS 2 versioning is 2.0.MBED_LIBRARY_VERSION

sg- added needs: CI and removed needs: review labels Nov 3, 2016

bridadan added do not merge and removed needs: CI labels Nov 4, 2016

infinnovation-dev force-pushed the mbed-version branch from 935d0f8 to 546da87 Compare November 4, 2016 21:55

infinnovation-dev force-pushed the mbed-version branch from 546da87 to 935d0f8 Compare November 4, 2016 22:05

infinnovation-dev mentioned this pull request Nov 4, 2016

Add mbed OS version macros #3207

Merged

geky changed the base branch from mbed-os-5.2 to master November 4, 2016 22:35

geky changed the base branch from master to mbed-os-5.2 November 4, 2016 22:38

bridadan closed this Nov 4, 2016

ccli8 mentioned this pull request Dec 15, 2016

[NUC472/M453] Support USB device #3365

Merged

Add mbed-os version macros #3096

Add mbed-os version macros #3096

Conversation

infinnovation-dev commented Oct 20, 2016

Description

Status

Migrations

theotherjimmy Oct 21, 2016

Choose a reason for hiding this comment

0xc0170 Oct 24, 2016

Choose a reason for hiding this comment

infinnovation-dev Oct 24, 2016

Choose a reason for hiding this comment

0xc0170 Oct 24, 2016

Choose a reason for hiding this comment

0xc0170 commented Oct 25, 2016

0xc0170 commented Oct 25, 2016

0xc0170 commented Oct 25, 2016

mbed-bot commented Oct 25, 2016

Result: FAILURE

bridadan commented Oct 25, 2016

mbed-bot commented Oct 25, 2016

Result: SUCCESS

Output

0xc0170 commented Oct 26, 2016

pan- commented Oct 26, 2016 • edited Loading

0xc0170 commented Oct 28, 2016

0xc0170 commented Oct 31, 2016

geky commented Oct 31, 2016

0xc0170 commented Nov 1, 2016

infinnovation-dev commented Nov 1, 2016

0xc0170 commented Nov 2, 2016

infinnovation-dev commented Nov 2, 2016

geky commented Nov 2, 2016

0xc0170 commented Nov 3, 2016

infinnovation-dev commented Nov 3, 2016

sg- commented Nov 3, 2016

sg- commented Nov 3, 2016

sg- commented Nov 3, 2016

mbed-bot commented Nov 3, 2016

sg- commented Nov 3, 2016

bridadan commented Nov 3, 2016

bridadan commented Nov 4, 2016

sg- commented Nov 4, 2016

mbed-bot commented Nov 4, 2016

bridadan commented Nov 4, 2016

mbed-bot commented Nov 4, 2016

Result: FAILURE

Output

bridadan commented Nov 4, 2016 • edited Loading

infinnovation-dev commented Nov 4, 2016

bridadan commented Nov 4, 2016

geky commented Nov 4, 2016

bridadan commented Nov 4, 2016

pan- commented Oct 26, 2016 •

edited

Loading

bridadan commented Nov 4, 2016 •

edited

Loading