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

Improve flags to allow subproject specific debug configuration #10

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

winterelo
Copy link

Hello there,

we have been using this library for a while now as a submodule in a student project of ours and it works like a charm. However our project has become enormous in size and relies on multiple submodules.

New Features

This pull request introduces two small changes to preprocessor directives:

  1. Add the RS_ prefix to the debug macro. This allows specifying that only the debug flag in the Reed-Solomon submodule is set and not for the whole project.
  2. Add the RS_NO_ASSERT macro. Our project will later run on an STM32L4 Microcontroller and therefore can only use freestanding libraries. With this flag we could disable including <assert.h> without having to set the debug flag.

Breaking changes

The first change might break some interfaces, that rely on the DEBUG macro. If this is not manageable i could also remove this renaming.

Also we think with the __CC_ARM you wanted to remove non-freestanding libraries when cross-compiling for an arm-microcontroller. In case this is true, let me suggest using the __STDC_HOSTED__ directive instead (see cppreference). This is set to 0 by the compiler when using a freestanding implementation and can be used to determine whether to include <assert.h> for a broader target than just cc_arm.

@winterelo
Copy link
Author

Hello again,

It has been a while so I am writing to check up on this matter. These features still are relevant to us and we would rather merge them here instead of hosting them in our own repository.
In case these changes are not acceptable in this form, I am open for suggestions.

Thank you in advance.

@mersinvald mersinvald merged commit 96b9a08 into mersinvald:master Dec 15, 2023
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.

2 participants