search

per1234 / PalatisSoftPWM

Software PWM library for Arduino
BSD 3-Clause "New" or "Revised" License
18 stars 5 forks source link

Compilation fails with the new Arduino IDE version 1.8.6 #7

Closed davorvr closed 6 years ago

davorvr commented 6 years ago

Both code examples in the repo fail at channel defintions with the new 1.8.6 version of the Arduino IDE. The 1.8.5 version works fine.

GCC was upgraded from 4.9.2 to 5.4.0 in the new Arduino IDE version so that's probably what to look at. Unfortunately, I lack the C++ knowledge to be able to find out what precisely changed between the two GCC versions to introduce this error.

per1234 commented 6 years ago

Thanks for the report. I'm aware of this issue and have already made a fix to the library that should resolve it but have not yet merged that fix to the master branch. You can download the development version of the library here: https://github.com/per1234/PalatisSoftPWM/tree/avr-boards-1_6_21-bug

I did run into another problem with using the library in a more complex sketch with avr-gcc 5.4.0 and wanted to also resolve that problem and then do some more thorough testing before doing a new release. I'll try to get that finished soon and will close this issue report at that time so you'll get a notification.

davorvr commented 6 years ago

Excellent! I apologize, I did not notice the other branch at all. Thank you!

In the interest of learning, a question popped into my head while looking at the commits in the other branch. In this commit, it seems you have a pointer declared as both volatile and const. If I may ask, what happens then, why was it necessary, and why not just switch from const to volatile instead of declaring as both?

per1234 commented 6 years ago

I apologize, I did not notice the other branch at all.

No apologies necessary. I'm not surprised you didn't notice the other branch, especially since I made a typo in the name (1.6.21 instead of 1.6.22). I was meaning to create an issue for this bug myself but at that time Arduino IDE 1.8.6 hadn't yet been released and I felt close to having things resolved so I didn't bother. Then I got busy with work deadlines and the 1.8.6 release happened before I got back to it.

In this commit, it seems you have a pointer declared as both volatile and const. If I may ask, what happens then, why was it necessary, and why not just switch from const to volatile instead of declaring as both?

I have no clue. To be honest, I don't really understand most of this library's code. The smart bits are all written by Palatis. I forked the library in order to try to make it more friendly to Arduino beginners and that work didn't require modifying the tricky code. Now these new bugs were introduced by the compiler update and it forced me to go muddling around trying to fix them, mostly through trial and error. I think I just slapped volatile on every instance of _channels and saw that the overoptimization bug went away. It's possible that not all those changes were necessary. I can't remember whether I checked. I spent more time than I had available trying to find a solution to the original bug you reported and then immediately hit the overoptimization one and another at the same time so I didn't have the time to work on it. Actually the only reason that branch was even pushed to GitHub was to run the Travis CI build on it. I plan to look more closely at everything before pushing it to the master branch.

davorvr commented 6 years ago

Ah, I understand. Thank you for the swift and detailed response!