Ar-zz-duboy / Arduboy

Core library for the Arduboy.
https://www.arduboy.com
Other
331 stars 96 forks source link

Add a #define for the library version #118

Closed MLXXXp closed 8 years ago

MLXXXp commented 8 years ago

A #define should be added, and maintained, in Arduboy.h (or core.h if that makes more sense) that gives the version of the library. I suggest _ARDUBOY_LIBVER as the name. This could be used by sketches for conditionally compiled code, so as to work with multiple non-compatible library versions.

The format should be the same as the ARDUINO define provided by the IDE to indicate its version:

E.g, for version 1.5.42 it would be: #define ARDUBOY_LIB_VER 10542 for 167.59.1 #define ARDUBOY_LIB_VER 1675901

For a sketch to work with library version 1.2.0 or higher, and still work with the version it's originally for, it would use:

#include <Arduboy.h>
Arduboy arduboy;
#if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200
AbPrinter text(arduboy);
#endif
// [...]
#if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200
text.print("Hello");
#else
arduboy.print("Hello");
#endif

Later, once the newer library version was well established and available, and it was unlikely anyone would still be using the older version, the sketch could be updated to remove the older version compatibility, if desired.

joshgoebel commented 8 years ago

I don't think we want to encourage coding to ALL versions of the lib like that, but I'm fine with the version number being defined. It should go in Ardudoy.h since that is the actual name of the included file and name of the library itself.

Shouldn't we also have LIB_VER_MAJOR, LIB_VER_MINOR, etc... or is someone suppose to do math if they need to figure that out?

joshgoebel commented 8 years ago

In your case it'd make mose sense to do something like this: (change names however)

#if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200
AbPrinter text(arduboy);
#define PRINTER text
#endif

PRINTER.print("hi");

Then you don't have to change your code everywhere. But again I don' think we should encourage this. We should distribute binaries or encourage people to build against a particular version - the intention being that these big breaking changes are a one time thing.

joshgoebel commented 8 years ago

Feel free to make a PR for this.

MLXXXp commented 8 years ago

I don't think we want to encourage coding to ALL versions of the lib like that

I absolutely agree. I envisioned this only being used to support transitions from the current API to a new one which would break the sketch. If, in the future, a third library with a new API came along, requiring additional changes to the sketch, the conditionals for the oldest library would be removed during these changes, leaving support only for the latest two APIs.

And I would encourage that once the newest library version is likely to have been installed by most of the users, the conditional code in the sketch be removed, with only support for the newest API remaining. This would force any "stragglers" to update.

The scenario that see this used for is:

MLXXXp commented 8 years ago

Shouldn't we also have LIB_VER_MAJOR, LIB_VER_MINOR, etc... or is someone suppose to do math if they need to figure that out?

They aren't necessary for the purpose I've described above. If you can see a use for them, they could be added. However, if they are needed only rarely, as you say, the users could just do the math on the combined value themselves.

The IDE doesn't provide this for its version.

MLXXXp commented 8 years ago

In your case it'd make mose sense to do something like this: (change names however)

#if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200
AbPrinter text(arduboy);
#define PRINTER text
#endif
PRINTER.print("hi");

Then you don't have to change your code everywhere.

Yes, that's actually the way I would do it. I didn't do that in my example because I thought you might reply with an objection to using a #define that way, similar to our EEPROM API discusion here. I'm really not sure when you think it's acceptable to use _#define_s to replace code and when you think it "makes it harder to understand what is happening".

For instance how would you feel if in one case it was a class function and in the other a standalone function?

#if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200
#define TONE tone
#else
#define TONE arduboy.tunes.tone
#endif

TONE(1000, 2000);
MLXXXp commented 8 years ago

I should note that once the sketch is only supporting libraries that contain the _#define ARDUBOY_LIBVER,

then instead of: #if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200

the sketch will just need to use: #if ARDUBOY_LIB_VER >= 10300

MLXXXp commented 8 years ago

PR #122 created.

joshgoebel commented 8 years ago

I also mentioned we could also add this to teh stable branch if it was worth the effort. merged your PR and closing this issue.

MLXXXp commented 8 years ago

Actually, for the purpose that ARDUBOY_LIBVER was added, there's no need to add it to the stable branch, or any previous version of the library. I forgot that if an undefined macro is used in a #if_ statement, a value of 0 is returned. You don't get an error.

This is will work fine if we're testing for a release version equal to or greater than a certain value. The returned 0 will cause the test to fail without an error. We don't care what the exact previous version number was. We just want to know if it's earlier than the version we're making the changes for.

This means that using #if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200 (as given as an example in previous comments) isn't required.

Sketches can just use: #if ARDUBOY_LIB_VER >= 10200 because for any library that doesn't have ARDUBOY_LIB_VER defined, it will be treated as having a value of 0, thus appearing to be version 0.0.0

joshgoebel commented 8 years ago

Interesting. Good to know.

On Mar 2, 2016, at 8:27 AM, Scott Allen notifications@github.com wrote:

Actually, for the purpose that ARDUBOY_LIB_VER was added, there's no need to add it to the stable branch, or any previous version of the library. I forgot that if an undefined macro is used in a #if statement, a value of 0 is returned. You don't get an error.

This is will work fine if we're testing for a release version equal to or greater than a certain value. The returned 0 will cause the test to fail without an error.

This means that using

if defined(ARDUBOY_LIB_VER) && ARDUBOY_LIB_VER >= 10200

(as given as an example in previous comments) isn't required.

Sketches can just use:

if ARDUBOY_LIB_VER >= 10200

because for any library that doesn't have ARDUBOY_LIB_VER defined, it will be treated as having a value of 0, thus appearing to be version 0.0.0

— Reply to this email directly or view it on GitHub.