next topic »

[Reply #45 on:]

Tim,
The preprocessor check you are using in LCDBitmap.h for trying to detect
the missing new liquidCrystal library is not using valid preprocessor syntax.
#ifdef does not support logical operations, so it isn't working the way you
intended it to work.
You will see the warning when using the 1.x IDE.

The sub device defines being checked seem to be for the purpose of including "LCD.h" but
the liquidCrystal.h in the new LiquidCrystal library already includes "LCD.h".
Since there is no need to include "LCD.h" again,
I would suggest avoiding the use of the sub device defines as it ties the bitmap library
to modifications and updates to the fm's library. i.e. if a new sub device is added
or the sub device define changes, this library breaks.

There are a couple of ways to handle it.
1) Do nothing instead
2) error off if neither the stock or fm's library is being used.

It depends on if you want to give an error if the user is using some other LCD library
other than stock LiquidCrystal or fm's library.

I would lean towards option 2 as it offers a meaningful error vs just breaking with
nothing but strange errors if they are using some other 3rd party LCD library.

It turns out that even including <LiquidCrystal.h> is problematic because when using another
3rd party LCD library (LiquidCrystalFast for example) the include path will not point to a directory that contains this header file
as the alternate library does not have it/use it.
Luckily the compiler doesn't treat a missing include file as a fatal error and
will generate a few more errors beyond this condition.
So while very ugly and will generate a few strange errors/warnings before the useful error message,
it is still possible to give the user the error to indicate that the bitmap library only works
with the new LiquidCrystal library.

Here is an alternative that works:

Code:

#ifndef LiquidCrystal_h // Using some library other than stock LiquidCrystal?
#ifndef _LCD_H_ // using some other library than fm's new LiquidCrystal?
    #error You must install New LiquidCrystal library to work with non-4bit projects: http:/bitbucket.org/fmalpartida/new-liquidcrystal/wiki/Home
#endif
#endif

This will work with the stock LiquidCrystal library or fm's library and if some other 3rd party
library is used, reports the error.

Just a minor nit, the declaration of "sample" in the examples creates a warning.
It needs to be declared as an unsigned int to eliminate that.

--- bill

[Reply #46 on:]

Tim,
The preprocessor check you are using in LCDBitmap.h for trying to detect
the missing new liquidCrystal library is not using valid preprocessor syntax.
#ifdef does not support logical operations, so it isn't working the way you
intended it to work.
You will see the warning when using the 1.x IDE.

However, you can do:

Code:

#if defined(FOO) && defined(BAR) && (!defined(BLETCH) || (BLETCH > 4))
#error "bogus"
#endif

[Reply #47 on:]

Tim,
The preprocessor check you are using in LCDBitmap.h for trying to detect
the missing new liquidCrystal library is not using valid preprocessor syntax.
#ifdef does not support logical operations, so it isn't working the way you
intended it to work.
You will see the warning when using the 1.x IDE.

However, you can do:

Code:

#if defined(FOO) && defined(BAR) && (!defined(BLETCH) || (BLETCH > 4))
#error "bogus"
#endif

#if is the "smarter" proper preprocessor directive choice as it supports logicals, bitwise, math, and even text comparisons,
but the point I was trying to make was that using the sub device defines and
the include of "LCD.h" is not really needed and has the potential to break should the new LiquidCrystal
library be expanded to other sub devices in the future.

--- bill

[Reply #48 on:]

Tim,
The preprocessor check you are using in LCDBitmap.h for trying to detect
the missing new liquidCrystal library is not using valid preprocessor syntax.
#ifdef does not support logical operations, so it isn't working the way you
intended it to work.
You will see the warning when using the 1.x IDE.

The sub device defines being checked seem to be for the purpose of including "LCD.h" but
the liquidCrystal.h in the new LiquidCrystal library already includes "LCD.h".
Since there is no need to include "LCD.h" again,
I would suggest avoiding the use of the sub device defines as it ties the bitmap library
to modifications and updates to the fm's library. i.e. if a new sub device is added
or the sub device define changes, this library breaks.
Just a minor nit, the declaration of "sample" in the examples creates a warning.
It needs to be declared as an unsigned int to eliminate that.

I get no warnings with Arduino 0023 nor 1.0.1.  Not sure what warnings I should be seeing.

The LCD.h include was added as suggested by fm.  However, it does appear that it's not needed.  Never questioned the requirement as I assumed it was needed.

I do see the error in my ifdef logic, I'll change that in a new release.

Making "sample" an unsigned int adds about 500 bytes to the compiled code size (because it loads the floating point library).  Not sure where you see a warning with this one either as I see no warning.  I could change it to "#define sample 4000", but that appears to work exactly the same as the "const".  More interested in why I'm not getting any warnings and why I should be using an unsigned int instead of a const or define as it's not really a variable, it's static.

Tim

[Reply #49 on:]

I get no warnings with Arduino 0023 nor 1.0.1.  Not sure what warnings I should be seeing.

Turn on "Show verbose output during" [] compilation
under [File]->[Preferences]
Then you will see the warning.

Making "sample" an unsigned int adds about 500 bytes to the compiled code size (because it loads the floating point library). 
Not sure where you see a warning with this one either as I see no warning.  I could change it to "#define sample 4000", but that appears to work exactly the same as the "const".  More interested in why I'm not getting any warnings and why I should be using an unsigned int instead of a const or define as it's not really a variable, it's static.

Tim

When you say "static" I assume you mean "not changing" vs a static declaration
as "static" to the compiler merely means that it is not visible to other modules.
It is independent of type. (char, int, unsigned int,  etc...)

I'm guessing that when you changed the variable type to "unsigned int" that you removed the "const"
declaration.
If so, then you are not getting the floating point library but rather
what you are seeing is the difference in optimization that is possible when
declaring the variable a "const" vs not const AND also not "static".

In this code, if it were declared "static" instead of "const" the compiler would have optimized it the same as if it were declared const.
To avoid the warning, the type needs be unsigned because both millis() and in particular currentMillis
are both unsigned.

The compiler complains about comparing a signed variable "sample" to an unsigned variable "currentMillis"
because the comparisons will probably not work as intended if sample is negative or overlows past 32767 and becomes
negative.

To keep things clean and avoid the warning, you should declare sample to be some unsigned type.
To get the best optimization, you will also need to declare it const or static.

Code:

const unsigned int sample = 4000;  // Sets how long to run each sample

--- bill

[Reply #50 on:]

Yes, I meant static as in not changing, not as a static declaration, and I get what you're talking about.

However, it seems that this:

Code:

#define sample 4000

would be an easier/better way to do it rather than using the const:

Code:

const unsigned int sample = 4000;

Less chance of the compiler complaining if the comparison variable type changes.  It seems either way are treated identically to the compiler.

Tim

[Reply #51 on:]

Yes, the compiler treats the two methods of setting the value the same. This seems to be because it is an optimising compiler. If I compile the classic Blink sketch, it is 1084 bytes. If I change "int led = 13;" to  "const int led = 13;" it is only 1076 bytes. I think it is not putting the number 13 into memory even though it was told to do so.

[Reply #52 on:]

"const" tells the compiler that the variable is never going to change. So it can treat it as a hard coded constant (but with type checking).
"static" tells the compiler that the variable will not be looked at or modified outside of this module.
Without either, the compiler must reference the actual memory for the value at least once inside a function.

When either is used and heavy optimization is enabled, both will generate the same code if the variable
is never modified as the compiler can look ahead and recognize that the variable is never modified and treat
it as a constant.

"volatile" tells the compiler that is must reference the actual memory every time the variable is referenced
as there is some other "non visible" code somewhere else that can be modifying the memory contents
of the variable at any time.

--- bill

[Reply #53 on:]

Yes, the compiler treats the two methods of setting the value the same. This seems to be because it is an optimising compiler. If I compile the classic Blink sketch, it is 1084 bytes. If I change "int led = 13;" to  "const int led = 13;" it is only 1076 bytes. I think it is not putting the number 13 into memory even though it was told to do so.

Reduction in code size is the reason I typically use #define or const.  For this particular issue it was originally a variable and then when I realized the number never changed I changed it to const to save code size.  I really should have made it to a #define for consistency, better understanding, and to avoid variable type issues like in this case.

Tim