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 misbehaving weak MillisecondTimer::delay function #171

Merged
merged 1 commit into from
May 17, 2016

Conversation

iromero91
Copy link
Contributor

For some reason when I try this library with my compiler [arm-none-eabi-g++ (15:4.9.3+svn231177-1) 4.9.3 20150529 (prerelease)] and the cmake toolchain it seems to cause the delay() function to return immediately (perhaps because it links to the wrong thing). The inline keyword is not very useful anyway for a function defined in the cpp file.

The problem was introduced in pull request #169.

@mikepurvis
Copy link
Contributor

+0

I see no harm in this if it fixes an issue for you. But it would be great to understand why the linker is doing the wrong thing, in case there's a deeper root cause.

@iromero91
Copy link
Contributor Author

iromero91 commented May 14, 2016

I'd like to know too, only thing i can think of is that I stumbled upon a GCC bug, I don't recall having a function with the inline attribute defined in the .cpp being undefined behavior, then again, the weak symbol machinery is not in the C++ standard.

Oh i found some evidence that it might be indeed a GCC quirk https://gcc.gnu.org/ml/gcc-help/2014-02/msg00162.html

To be honest I think the best way going forward for this kind of stuff is to do overrides at the source level and switch from globally built library to something that can be inserted in your project as a git submodule.

@mikepurvis
Copy link
Contributor

Compile time ifdefs is certainly a standard practice in the embedded C world— that's for sure how libraries like FreeRTOS and LwIP are configured. That said, I really appreciate the amount of configuration that is possible in stm32plus using templates to still deliver relatively compact binaries.

This approach makes library a good fit for being able to use pre-compiled binaries of stm32plus, which works well in my company's projects, and that ability breaks down once there are project-specific switches that need to be set at compile time.

Indeed, I even wonder if it might be possible to eliminate specifying the HSE/HSI at compile time by having a weakref function return that value, which can be overridden by the project, though that discussion is getting out of the scope of this ticket. :)

@andysworkshop
Copy link
Owner

I'd like to see the linker output that shows what's actually been linked to when the problem occurs but nevertheless I see no problem with the pull request as-is. Merging.

@andysworkshop andysworkshop merged commit 290d523 into andysworkshop:master May 17, 2016
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