Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix integer warnings #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Rotzbua
Copy link
Contributor

@Rotzbua Rotzbua commented Feb 1, 2018

e.g.:
LoRaDuplex.ino:47:33: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

e.g.:
LoRaDuplex.ino:47:33: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
@sandeepmistry
Copy link
Owner

@Rotzbua thanks for submitting, what board are you building for?

When I compile for Arduino Uno on IDE 1.8.5 I don't see any warnings.

@Rotzbua
Copy link
Contributor Author

Rotzbua commented Feb 5, 2018

I usually switch the arduino ide to the highest warning level, then you will get the warning.

The (byte)LoRa.read is just for sanity, because int type is returned and do not fit into byte.

@sandeepmistry
Copy link
Owner

I usually switch the arduino ide to the highest warning level, then you will get the warning.

Thanks I missed that, for the Uno I now get (without this PR):

/Users/smistry/Documents/Arduino/libraries/LoRa/examples/LoRaDuplex/LoRaDuplex.ino: In function 'void loop()':
/Users/smistry/Documents/Arduino/libraries/LoRa/examples/LoRaDuplex/LoRaDuplex.ino:47:31: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if (millis() - lastSendTime > interval) {
                               ^

The (byte)LoRa.read is just for sanity, because int type is returned and do not fit into byte.

@tigoe any feedback on those changes? Do you think casting to byte is unfriendly to beginners?

@Floessie
Copy link

Floessie commented Feb 5, 2018

Do you think casting to byte is unfriendly to beginners?

Those explicit casts to byte on assignment aren't necessary. So the question shouldn't arise. Just my 2¢.

Best,
Flössie

@Rotzbua
Copy link
Contributor Author

Rotzbua commented Feb 5, 2018

Normaly I use clion, it always warns about implicit type conversation. Implicit type conversations are often reasons for bugs, so I personally do not like them.

It seems that -Wall do not include -Wconversion to enable the warning.

@Floessie
Copy link

Floessie commented Feb 6, 2018

Implicit type conversations are often reasons for bugs, so I personally do not like them.

Fair enough.

It seems that -Wall do not include -Wconversion to enable the warning.

That's for a reason. Quoting:

Implicit conversions are very common in C. This tied with the fact that there is no data-flow in front-ends (see next question) results in hard to avoid warnings for perfectly working and valid code. Wconversion is designed for a niche of uses (security audits, porting 32 bit code to 64 bit, etc.) where the programmer is willing to accept and workaround invalid warnings. Therefore, it shouldn't be enabled if it is not explicitly requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants