ldab / KXTJ3-1057

KXTJ3-1057 Tri-axis Digital Accelerometer - Arduino Library
MIT License
14 stars 8 forks source link

#define preprocessor directives do not work from sketch context #6

Closed nomakewan closed 1 year ago

nomakewan commented 1 year ago

Just a heads up that the example code and any similar code placed into a .ino sketch to define a directive within this library will not pass on to the library correctly.

If you define both LOW_POWER and HIGH_RESOLUTION in a sketch, the error in the header for KXTJ3-1057 will be thrown as expected because the header does accept the directive, and it is the header itself throwing the error. However, if you define HIGH_RESOLUTION by itself in the sketch, this definition will not be retained for KXTJ3-1057.cpp when the Arduino IDE compiles the sketch and its linked libraries. You can check this without any hardware at all by simply modifying your local copy of the library to do something that should cause the library to fail to compile if that branch of code is reached, such as removing a semicolon from the end of a line under the #ifdef HIGH_RESOLUTION context.

A good example of where and how to do this would be here in KXTJ3::applySettings:

#ifdef HIGH_RESOLUTION
    dataToWrite = 0xC0
#endif

One would think that writing #define HIGH_RESOLUTION in the sketch would cause the compilation to fail due to the missing semicolon, but it does not.

You can further prove this by breaking the 'else' part of the start-up delay check. If HIGH_RESOLUTION is defined, that 'else' should never get touched. Yet you can remove semicolons from anywhere in the HIGH_RESOLUTION section with impunity, but if you remove them from the 'else' area the compilation will fail, proving that it is compiling using the LOW_POWER definitions instead.

This appears to be an issue in the Arduino IDE, though many replies on the forums claim this is both intentional behavior and standards-compliant. I have tried several local experiments to somehow allow a definition in the sketch to be passed to the KXTJ3-1057.cpp file but was unable to find a solution short of just defining HIGH_RESOLUTION in KXTJ3-1057.h directly rather than placing the define in the sketch.

ldab commented 1 year ago

Good to know, I can confirm it seems to be an issue, don't plan to fix it myself tho :)

nomakewan commented 1 year ago

I found it completely by accident while working on an unrelated library, but was already in the middle of adding functionality to this one. The easiest solution I can see would be to change it from a preprocessor directive to a variable passed to begin() (with a default value, of course). Debug mode, too. Since I'm already working on other features it would be fairly trivial to make this change if that's acceptable.

Debug mode would default to false, but what about resolution? Would you prefer that the library defaults to low-resolution unless the user explicitly sets HIGH_RESOLUTION to true, as with the preprocessor directive?

nomakewan commented 1 year ago

I noticed that some other libraries are getting around this limitation by putting the entirety of their code into a header file and including that in a sketch instead. Here is one such example. By dumping the entirety of the code into a single header file and including that in the .ino instead, the #define directive is handled 'properly' by the Arduino IDE.

However, I still think that it would be better to simply expose the resolution and debug options as optional parameters in begin().

Thoughts?