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

min / max macros in opl2.h cause issues when using opl2.h with other libraries #73

Closed
isnotinvain opened this issue Dec 20, 2020 · 4 comments · Fixed by #77
Closed

min / max macros in opl2.h cause issues when using opl2.h with other libraries #73

isnotinvain opened this issue Dec 20, 2020 · 4 comments · Fixed by #77

Comments

@isnotinvain
Copy link
Contributor

Filing this so I don't forget -- I'll try and send a PR with a fix soon.

I'm not a C++ expert but I did some googling after hitting some compiler errors when I tried to use std::min in a file that includes OPL2.h and found that defining min/max macros is discouraged (for this reason).

This workaround works:

#include <OPL2.h>
#undef min
#undef max

... rest of your code... 

I think probably std::min and std::max could be used instead, at least for raspberry pi. Or this macro could move to the relevant cpp files instead.

@DhrBaksteen
Copy link
Owner

In the library and examples these functions are only used to clamp values to a valid range. Using std:: is not an option for Arduino due to its memory limitations. The upcoming library update contains a helper fucntion clamp to take care of this and that's also where the Arduino / Raspberry Pi / others distinction will be made to prevent spreading complier directives throughout the code in order to keep it simple and readable.

@isnotinvain
Copy link
Contributor Author

ok sounds good, thanks!

DhrBaksteen added a commit that referenced this issue Jan 20, 2021
Possible fix for #73 still to be tested on all platforms other than Raspberry Pi.
@DhrBaksteen
Copy link
Owner

Please reopen if the issue persists

@isnotinvain
Copy link
Contributor Author

Great, thanks! I'll back out my undefines and see how it goes

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 a pull request may close this issue.

2 participants