using current Arduino IDE, FirmataPlusMrYsLab.ino gives warnings when compiled

[ericwertz]

When compiling the FirmataPlus library, at least two different (types of) warnings are issued. It's not clear if they were issued by previous versions of the compiler tools, but they are now.

The first is the unexpected, unadorned URL information that's trying to be a comment after a #include directive. In particular (but not the only instance of, in FirmataPlusMrYsLab.ino),

#include <AdaEncoder.h> https://code.google.com/p/adaencoder/downloads/detail?name=adaencoder-v0.7beta.zip

I don't ever remember this being legal C, and the current compiler does complain about it. Changing this to a real //-style comment eliminates this warning.

The other warning, also in FirmataPlusMrYsLab.ino, is:

C:\Users\ericw\Documents\Arduino\FirmataPlusMrYsLab\FirmataPlusMrYsLab.ino: In function 'void sysexCallback(byte, byte, byte*)':

FirmataPlusMrYsLab.ino:849:36: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]

       printData("argc = ", argc) ;

                                ^

This warning can be squelched simply by casting the string to (char *).

[ericwertz]

Sorry, to clarify -- this is when compiling for AVR. Unclear what the different flavors of gcc-targets do, but they shouldn't be architecture-related.

[MrYsLab]

Yes, the toolchain was "updated" along the way and these warning started appearing. Since working with the Firmata sketch is, well, let us say, unpleasant, and since these were "just warnings", I decided to leave things as-is, since I knew the code works. I did document the warnings in the WiKi.

I will classify the warning into 2 groups. Ones that I introduced and those that are contained in the libraries I am using (the Adafruit encoder library and the oopinintchange library that Adafruit library requires.)

If I ever go into the Firmata sketch code again, I will clean up my warnings since that is simple to do, but I won't be touching the external libraries. The maintainer for these libraries has been unwilling to make any changes. These libraries are quite old and have not been touched in 4 or 5 years. So, since the sketch will produce warnings, no matter what I change in my code, I made this a very low priority item.

When I ran a software group, I always insisted that we have no warnings when publishing code, so I can understand where you are coming from.

[MrYsLab]

I took a quick look at warnings, and the level of reporting set in the IDE preferences dialog may open up a Pandora's box of discussion. Taking StandardFirmata as an example, if "Compiler warnings" is set to "All", a whole slew of warnings is issued. So the question (as philosophical as it might be) is, what is the appropriate level of warnings to check for? Why would the "Default" level be the correct one as opposed to any of the other levels? Why isn't a stricter level the correct one and how many libraries could be compiled at stricter levels without some warning be issued?

At the default level, I could easily fix my 3 or 4 warnings, but personally, don't think that warrants the effort for a new release. If a user doesn't need the encoder support, they can use the FirmataPlus32u4 sketch which is identical to FimrataPlus, except it has the encoder support removed. The 32u4 version works on the other supported Arduino boards (Uno, Mega, etc) and was created because at some point when the AVR toolchain was changed by the Arduino folks, and it no longer could compile the encoder libraries when the Leonardo was selected as the board type. In the past, it didn't complain and I had a single sketch. If the user then tried to use encoder support on a Leonardo, they would always get a value of zero back, and this was documented by me.

So to make a long story short (or longer), I would be willing to change the FirmataPlus code at some point in the future in order to not generate warnings in my code when compiled to a default warning level, but the Adafruit library would still generate warnings. So I am not sure if these changes warrant the effort in creating a new release, that in fact, only "fixes" warnings that have no effect on the operation of the code, and only fix the warnings generated within the code I have written, but not the code that was written by others (the Adafruit encoder support).

[ericwertz]

Ah, glad you mentioned that. Yeah, I could go on a rant about warning levels myself. When we were using the Arduino IDE I'd tell students that all warnings were their friends and that they should crank it up most/all of the way. ​ Some libraries are pretty horrible. Some may have been written before either the default settings were changed, or the default set of warnings changed. Even the Arduino core had warnings at the default level for periods of time in the past.​ ​ I'll tell you why getting rid of errors at the default level is very important. I've gotten a lot of emails about why the example "doesn't compile right" because of the warnings. I even caught one user deciding that the bundled StandardFIrmataPlus was better than yours because he got "errors" when he used yours. "Interesting" judgement call on his part.​ ​ At the beginner level, any noise is a panic situation, so I painstakingly avoid anything that creates unnecessary stress, questions, or "impure" guidance. I personally feel like we have to be on our best behavior in front of them because then they start to slide down the slippery slope. I have a no-warnings policy. At the early stages of instruction, I don't find that it's a difficult cross to bear. However, the real world usually quickly rears its ugly head and they inevitably start running into others' libraries that give a bunch of warnings, for whatever historical/hysterical reasons. My $0.02 on that.​ ​ I'm certainly not going to tell you how to write your own code. But if any library were gross enough, I'd probably have to either fork it or create a local copy for students and clean it up myself, for any given semester. There are probably so many conditional-compilation combinatorics, cross-compilers in use now, and tool updates that it's understandable tha warnings creep in. Personally all that I care about is that the Uno builds are clean, at this point in time.​ ​ Interestingly, I just checked my compiler settings and it's telling me it's set to "None" (not even Default) ! I sure don't remember ever doing that, except maybe for some type of test, and I must have forgotten to set it back. I'm aghast that it's set there now. I'm glad you brought this up.​ ​ As far as the AdaEncoder library is concerned, my guess is that LadyAda would be happy to take updates that clean-up warnings. I'll look into that myself. When I compile your .ino, I'm only seeing those two errors of yours though, and none from anywhere else. Again, for the Uno. If I switch to the Leonard, I get even more warnings from yours, plus more from other libraries. I'll have to let the Leonardo warriors fight that battle. At one point the Arduino team discontinued the Leonardo, so it's unsurprising there was a period of time in which code for that platform rotted even faster.​ ​ We even used to (try to) teach our students how "PORT-style" programming (using the PORT/PIN/DIR registers) works, but we finally gave up on that last year because there were too many other more important things that they weren't learning about. We traded that week for learning about state machines instead. And, of course, PORT-level code for the Uno is portable to almost exactly nowhere else -- so out it went. I'm always making the case to the instructor that these guys need to be productive and be able to show as much as possible for their work (to themselves and employers) that we're moving away from stuff that's low-level. I would *not* make the same choice for CS/EE/EECS/CompE majors, though.​ ​ You should do whatever you want with your code -- I'll leave it at that. I'm exceedingly happy to have it out there and that our students are getting value out of it. Thanks again.​ ​ -e​

…

________________________________ From: Alan Yorinks <notifications@github.com> Sent: Sunday, October 21, 2018 7:39 AM To: MrYsLab/pymata-aio Cc: Eric B. Wertz; Author Subject: Re: [MrYsLab/pymata-aio] using current Arduino IDE, FirmataPlusMrYsLab.ino gives warnings when compiled (#85) I took a quick look at twarnings, and the level of reporting set in the IDE preferences dialog may open up a Pandora's box of discussion. Taking StandardFirmata as an example, if "Compiler warnings" is set to "All", a whole slew of warnings is issued. So the question (as philosophical as it might be) is, what is the appropriate level of warnings to check for? Why would the "Default" level be the correct one as opposed to any of the other levels? Why isn't a stricter level the correct one and how many libraries could be compiled at stricter levels without some warning be issued? At the default level, I could easily fix my 3 or 4 warnings, but personally don't think that warrants the effort for a new release. If a user doesn't need the encoder support, they can use the FirmataPlus32u4 sketch which is identifical to FimrataPlus, except it has the encoder support removed. The 32u4 version works on the other supported Arduino boards (Uno, Mega, etc) and was created because at some point, when the AVR toolchain was changed by the Arduino folks, and it no longer could compile the encoder libraries when the Leonardo was selected as the board type. In the past, it didn't complain and I had a single sketch. If the user then tried to use encoder support on a Leonardo, they would always get a value of zero back, and this was documented by me. So to make a long story short (or longer), I would be willing to change the FirmataPlus code at some point in the future in order to not generate warnings in my code when compiled to a default warning level, but the Adafruit library would still generate warnings. So I am not sure if these changes warrant the effort in creating a new release, that in fact only "fixes" warnings that have no effect on the operation of the code, and only fixes the warnings generated within the code I have written, but not the code written by others (the Adafruit encoder support). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#85 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AALc9aHIL_R7jhJYnasgLqYzkAS4E-z6ks5unIc9gaJpZM4XyQiV>.

[MrYsLab]

Unfortunately, the adaencoder library is not supported by LadyAda, but by Mike Schwager[https://github.com/GreyGnome]. He also maintains the ooPinChangeInt library, which is also used for rotary encoder support. When we first started getting these warnings, I had contacted him directly, and he showed no interest in going back in and cleaning things up.

I guess a fork with cleanup is a reasonable solution since those libraries haven't been maintained for years. My only concern is if there is some squirrely code that I "fix" may introduce errors. I will do a quick check to see what is involved to clean things up, but if it turns out to be a rewrite, I will probably abandon trying to fork and fix the code.

How important is rotary encoder support for you? If it is not important, then FirmataPlus32u4 can be used with the Uno (and other boards as well). It is exactly the same as FirmataPlus but with the encoder stuff removed.

If the FirmataPlus32u4 solution is acceptable, I would be willing to change the code to clean up the 2 warnings that need the casts. This would give a compile with no warnings when the IDE is set to default level, keep the students and you happy. You would just need to direct them to use the 32u4 version.

Is the 32u4 solution acceptable?

[MrYsLab]

An update - I can easily fix the warnings for default level of warnings (I think). However, it appears that for pymata-aio, rotary encoder support is not functioning (but it does for the older PyMata library).
I will open an issue for rotary encoder support.

I will get back to you here when I solve the rotary encoder issue. This may be several days.

[ericwertz]

I only got two warnings, for the extra cruft after the two #include statements, and in one call to PrintData() that just needs a (char *) on the front of it -- from what I remember. tkx, -e

…

________________________________ From: Alan Yorinks <notifications@github.com> Sent: Monday, October 22, 2018 12:12 PM To: MrYsLab/pymata-aio Cc: Eric B. Wertz; Author Subject: Re: [MrYsLab/pymata-aio] using current Arduino IDE, FirmataPlusMrYsLab.ino gives warnings when compiled (#85) An update - I can easily fix the warnings for default level of warnings (I think). However, it appears that for pymata-aio, rotary encoder support is not functioning (but it does for the older PyMata library). I will open an issue for rotary encoder support. I will get back to you here when I solve the rotary encoder issue. This may be several days. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#85 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AALc9fYJb-5qiU9qq31lYhg3wfXUrkMCks5unhibgaJpZM4XyQiV>.

[MrYsLab]

Code has been modified to alleviate all warnings, both in the FirmataPlus code as well as in the supporting libraries.

[MrYsLab]

I just pushed out a new libraries.zip file. This should fix all of the warnings.