search

OpenCyphal / libcanard

A compact implementation of the Cyphal/CAN protocol in C for high-integrity real-time embedded systems
http://opencyphal.org
MIT License
339 stars 195 forks source link

Use a separate header file for assert macros #71

Closed adolfogc closed 5 years ago

adolfogc commented 5 years ago

Hello,

I need to override the CANARD_ASSERT macro in a project in which I included Libcanard as a Git submodule. I don't want to make a separate copy of canard.h just to override this macro. A convenient solution to this problem is to have a separate canard_assert.h header for the user to provide alternative macro definitions. This header would be included by canard.h. A default implementation would be provided, and the user could use a custom one if he/she wished to provide an alternative definition of the macros.

What do you think?

Best regards, Adolfo

pavel-kirienko commented 5 years ago

I recall someone in the past brought up a similar suggestion for libuavcan, saying that it would be nice to have a config header for preprocessor overrides. We ended up not changing anything because one can always override things via the compiler's command line; e.g. -DCANARD_ASSERT=assert for Clang or GCC. Does that not work for you?

adolfogc commented 5 years ago

Hi Pavel,

Thanks for your reply.

Does that not work for you?

The problem is that I need to use another macro, Q_ASSERT, which is defined as below:

#define Q_ASSERT(test_) ((test_) \
        ? (void)0 : Q_onAssert(&Q_this_module_[0], (int_t)__LINE__))

Source: https://github.com/QuantumLeaps/qpc/blob/master/include/qassert.h#L142

As far as I know, using -DCANARD_ASSERT=Q_ASSERT wouldn't work because another header file, that in which Q_ASSERT is defined, would still need to be included in canard.h.

pavel-kirienko commented 5 years ago

Indeed. You could instruct the compiler to pre-include a specific file, but that would probably be suboptimal. Seems like we need a config header, not just for assertions, but for all compile-time options (of which there are currently only assertions, but this may change in the future). canard_build_config.h should work, I suppose? Any objections? @adolfogc would you be able to implement that?

adolfogc commented 5 years ago

@pavel-kirienko Sounds good. I've implemented those changes in #72. Please review them.