stream: Support zephyr/net_buf.h #9
Conversation
include/cobs/stream.h
Outdated
| #include <ncs_version.h> | ||
| #endif | ||
|
|
||
| #if KERNEL_VERSION_NUMBER < 0x30300 |
| #include <net/buf.h> | ||
| #else | ||
| #elif NCS_VERSION_NUMBER < 0x20800 && KERNEL_VERSION_NUMBER < 0x40000 |
There was a problem hiding this comment.
even if there's no NCS_VERSION_NUMBER defined, as long as 3.1.0 <= KERNEL_VERSION_NUMBER < 4.0.0 this header should be included.
I also don't get the reason on why NRF SDK version should be involved in this. Does the NRF SDK modify the nebuf header path?
There was a problem hiding this comment.
the nrf sdk cherry picks major changes into previous versions of zephyr. So currently, their zephyr version reports 3.7.99, but they already moved that header file because they cherry picked 4.0.0 stuff.
There was a problem hiding this comment.
I see now, this approach can make things confusing. Please add this info in the commit description
There was a problem hiding this comment.
this doesn't seem correct for the case where NCS_VERSION_NUMBER is not defined.
IMO, the whole thing should look like:
#ifdef NCS_VERSION_NUMBER
// do checks based on NCS_VERSION_NUMBER
#else
// do checks based on KERNEL_VERSION_NUMBER
#endif
There was a problem hiding this comment.
why not? if it's not defined it evaluates to 0 and 0 is smaller than 0x20800, effectively removing the check due to the &&
There was a problem hiding this comment.
but <zephyr/net/buf.h> is the header you want to include if kernel is <4.0.0(and also > than 3.1.0). If expression evaluates to 0 because NCS_VERSION_NUMBER is not defined, then you will include <zephyr/net_buf.h>
There was a problem hiding this comment.
are you sure?
NCS_VERSION_NUMBER < 0x20800 && KERNEL_VERSION_NUMBER < 0x40000
= 0 < 0x20800 && 0x30700 < 0x40000
= 1 && 1
= 1
There was a problem hiding this comment.
you're right. I was under the wrong impression that (NCS_VERSION_NUMBER < 0x20800) evaluates to 0 if NCS_VERSION_NUMBER is not defined. But you already explained above. Sorry!
While at it, also simplify the existing check. We have to check sdk-nrf versions separately, because they cherry-pick breaking changes into their zephyr-fork.
No description provided.