Skip to content

Comments

update nonlinearcircuits; needs better fix for max macro#586

Merged
falkTX merged 3 commits intomainfrom
update/nonlinearcircuits
Oct 21, 2023
Merged

update nonlinearcircuits; needs better fix for max macro#586
falkTX merged 3 commits intomainfrom
update/nonlinearcircuits

Conversation

@dromer
Copy link
Collaborator

@dromer dromer commented Oct 1, 2023

Builds, but not pretty "fix" for that macro definition.

Adds 5 new modules.

Problem without the macro is:

nonlinearcircuits/src/NLCShared.hpp: In function ‘float SlothLedBrightness(float)’:
MockbaModular/src/MockbaModular.hpp:32:37: error: expected unqualified-id before ‘(’ token
   32 |         #define max(a,b)            (((a) > (b)) ? (a) : (b))
      |                                     ^
nonlinearcircuits/src/NLCShared.hpp:125:17: note: in expansion of macro ‘max’
  125 |     return std::max(0.0f, std::min(1.0f, v / 2.0f));
      |                 ^~~

@cosinekitty
Copy link
Contributor

@dromer I'm glad to see someone is bringing the NLC Sloth modules into Cardinal! As their author, please let me know if I can do anything to help. Hopefully it is a simple process.

@falkTX
Copy link
Contributor

falkTX commented Oct 1, 2023

the use of the max macro is not needed, and in fact unwanted as it breaks std::max usage.
Rack/Cardinal is C++ so the std::max function is available to use, please use that instead of C-style macros.

@cosinekitty
Copy link
Contributor

Builds, but not pretty "fix" for that macro definition.

Adds 5 new modules.

Problem without the macro is:

nonlinearcircuits/src/NLCShared.hpp: In function ‘float SlothLedBrightness(float)’:
MockbaModular/src/MockbaModular.hpp:32:37: error: expected unqualified-id before ‘(’ token
   32 |         #define max(a,b)            (((a) > (b)) ? (a) : (b))
      |                                     ^
nonlinearcircuits/src/NLCShared.hpp:125:17: note: in expansion of macro ‘max’
  125 |     return std::max(0.0f, std::min(1.0f, v / 2.0f));
      |                 ^~~

Would it help if I updated these to use if statements instead of min and max? Michael Hetrick gave me commit rights to the repo. Then you could update the submodule and these warnings would be fixed.

@falkTX
Copy link
Contributor

falkTX commented Oct 1, 2023

hmm actually the issue is on the MockbaModular modules, that is where the macro comes from. so we should modify that part of the code. the NLC modules seem fine

@dromer
Copy link
Collaborator Author

dromer commented Oct 1, 2023

Yeah the issue is NOT with NLC but the conflict is introduced by Mockba over here: https://github.com/MockbaTheBorg/MockbaModular/blob/479d2c8007b2087cdf557a491df25c5b85784a96/src/MockbaModular.hpp#L31-L33

It just arises in NLC because its build is handled alphabetically and comes after Mockba ;)

@falkTX
Copy link
Contributor

falkTX commented Oct 21, 2023

@dromer lets go for this, just one request: instead of "define max max" use "#undef max" right after the mockba includes, there is already a "#undef min" so just add it above this line.
thanks

@dromer
Copy link
Collaborator Author

dromer commented Oct 21, 2023

Hah yes indeed, totally missed that one :D

@falkTX falkTX merged commit b941d7d into main Oct 21, 2023
@falkTX falkTX deleted the update/nonlinearcircuits branch October 21, 2023 13:18
@falkTX
Copy link
Contributor

falkTX commented Oct 21, 2023

thanks again!

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