shujaat: [SOLVED] Volatile Qualifier

User avatar
mrburnette
Posts: 2223
Joined: Mon Apr 27, 2015 12:50 pm
Location: Greater Atlanta
Contact:

Re: Volatile Qualifier

Post by mrburnette » Thu Jan 18, 2018 8:22 pm

stevestrong wrote:
Thu Jan 18, 2018 7:24 pm
<...>
EDIT
There are 548 hits in 47 files for "__io" of Arduino_STM32 core, STM32F1 directory.
The cleanest solution would be to replace all those occurrences by "__IO" (make it uppercase) like in Teensy core, or by "volatile".
Steve,

I really hate myself (as I fight core changes usually) but in this instance I agree with you. Appears that Paul has been down this road already.

Ray

shujaat
Posts: 14
Joined: Thu Jan 18, 2018 7:47 am

Re: Volatile Qualifier

Post by shujaat » Fri Jan 19, 2018 5:44 am

stevestrong wrote:
Thu Jan 18, 2018 7:24 pm
Well, guys, I've found the issue.
Very simple.
Two files from the gcc pakage uses "__io" as input parameter for some functions.

This is just an unfortunate coincidence that the Arduino_STM32 core uses the same term to define volatile...

The Teensy core is not sensitive to this because it uses the same term in uppercase.

As soon as I renamed all occurences of this term to "__ios" in the two files

Code: Select all

local_facets.h
local_facets.tcc
in directory

Code: Select all

Arduino15\packages\arduino\tools\arm-none-eabi-gcc\4.8.3-2014q1\arm-none-eabi\include\c++\4.8.3\bits
the sketch compiled fine.

Code: Select all

Sketch uses 43968 bytes (67%) of program storage space. Maximum is 65536 bytes.
Global variables use 4376 bytes (21%) of dynamic memory, leaving 16104 bytes for local variables. Maximum is 20480 bytes.
EDIT
There are 548 hits in 47 files for "__io" of Arduino_STM32 core, STM32F1 directory.
The cleanest solution would be to replace all those occurrences by "__IO" (make it uppercase) like in Teensy core, or by "volatile".
Congratulations! I tested this too and it works flawlessly!
Moreover thanks for finding the bug, hopefully the core will improve with time and become more useful.
Thank you so much!

shujaat
Posts: 14
Joined: Thu Jan 18, 2018 7:47 am

Re: Volatile Qualifier

Post by shujaat » Fri Jan 19, 2018 5:55 am

mrburnette wrote:
Thu Jan 18, 2018 8:22 pm
stevestrong wrote:
Thu Jan 18, 2018 7:24 pm
<...>
EDIT
There are 548 hits in 47 files for "__io" of Arduino_STM32 core, STM32F1 directory.
The cleanest solution would be to replace all those occurrences by "__IO" (make it uppercase) like in Teensy core, or by "volatile".
Steve,

I really hate myself (as I fight core changes usually) but in this instance I agree with you. Appears that Paul has been down this road already.

Ray
Blindly advocating core will not make it great. We should welcome not only this but all those changes which will make it more cleaner and powerful.
Thanks for being patient and cooperating.

stevestrong
Posts: 2067
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany
Contact:

Re: [SOLVED] Volatile Qualifier

Post by stevestrong » Fri Jan 19, 2018 9:58 am

Just as a small note, making these replacements will not make the core better, nor cleaner or powerful.

Again: this issue is not a "core" issue, but an unfortunate coincidence between core, gcc and library.
We could blame gcc for using this term. Me, I would not come to the idea to use such a term as input parameter. Maybe with one underscore (_io) but definitely not with two underscores.
And we also could blame those who "misuse" this lib with this core because it was clearly mentioned that was not tested with it.

The question is: do we really need to do those replacements? It is really a huge effort.
Or it would suffice to leave a note (in Arduino_STM32 wiki or library readme file) to warn those who intend to use this lib what they have to do?
I would opt for the later.

shujaat
Posts: 14
Joined: Thu Jan 18, 2018 7:47 am

Re: [SOLVED] Volatile Qualifier

Post by shujaat » Fri Jan 19, 2018 1:04 pm

Folks at the teensy have used __IO for a better reason and I would suggest we should do the same. By the way if you name it coincidence or whatever else, it WILL affect the users and it WILL be called an issue by the users!

stevestrong
Posts: 2067
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany
Contact:

Re: [SOLVED] Volatile Qualifier

Post by stevestrong » Fri Jan 19, 2018 1:24 pm

@shujaat, you are welcome to submit a pull request on Github which solves this issue.
I am offering as volunteer for a review.
And I think Roger would be more than happy to have that PR.

zmemw16
Posts: 1685
Joined: Wed Jul 08, 2015 2:09 pm
Location: St Annes, Lancs,UK

Re: [SOLVED] Volatile Qualifier

Post by zmemw16 » Fri Jan 19, 2018 2:26 pm

there's a lot of grass to hide more snakes in ?
i.e. as in are there any others similar that might bite us ?
stephen

User avatar
Pito
Posts: 1742
Joined: Sat Mar 26, 2016 3:26 pm
Location: Rapa Nui

Re: [SOLVED] Volatile Qualifier

Post by Pito » Fri Jan 19, 2018 3:01 pm

fyi - replacing 1288x __io with __IO in my older local Arduino_STM32 repo took 14secs (Notepad++, Find and Replace in Files).
Pukao Hats Cleaning Services Ltd.

User avatar
mrburnette
Posts: 2223
Joined: Mon Apr 27, 2015 12:50 pm
Location: Greater Atlanta
Contact:

Re: [SOLVED] Volatile Qualifier

Post by mrburnette » Fri Jan 19, 2018 3:15 pm

I am on Android at the moment and away from my home lab, do we know what the Neucleo core code has done? That is, will it behave similar? If so, I believe Roger and fpiSTM should probably discuss privately and make a collective recommendation.

Ray

aster
Posts: 120
Joined: Thu Mar 30, 2017 2:41 pm
Location: bella italy
Contact:

Re: [SOLVED] Volatile Qualifier

Post by aster » Fri Jan 19, 2018 3:41 pm

it is great to know that steve tracked the problem down and found a wordaround for it! :D

IMHO since i am a lover of the clean code, i would do as pito did, if it is easy, it works, why don't change it?
i always think that this is work we are cut off from the future

Post Reply