search

FreeRTOS / FreeRTOS-Plus-TCP

FreeRTOS-Plus-TCP library repository. +TCP files only. Submoduled into https://github.com/FreeRTOS/FreeRTOS and various other repos.
MIT License
125 stars 149 forks source link

Add Kernel Version Check #1117

Closed HTRamsey closed 3 months ago

HTRamsey commented 3 months ago

Adds a check for minimum supported kernel version

AniruddhaKanhere commented 3 months ago

@ActoryOu should we be including the kernel headers here https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/df5aed9e58a72e2489b6c1936788885672c62981/test/build-combination/Header_Self_Contain/headerSelfContain.cmake#L21 before compile options as the build check is failing there.

ActoryOu commented 3 months ago

@ActoryOu should we be including the kernel headers here

https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/df5aed9e58a72e2489b6c1936788885672c62981/test/build-combination/Header_Self_Contain/headerSelfContain.cmake#L21

before compile options as the build check is failing there.

Sorry for late response. Yeah, that helps. And @HTRamsey created a better solution by #1120. Let's do it that way.

tony-josi-aws commented 3 months ago

@HTRamsey

Thanks for this PR. But to me, it seems like the kernel version check is not adding much value here, as without the check as well, the compilation should fail if the user is not using the right kernel version.

Another reason is that, though event groups are added with kernel version v8.1, task notifications (which were added with v8.2) could be used in the network interface or with ipconfigSELECT_USES_NOTIFY. For users who do not use task notification, it doesnt make much sense to have minimum version as v8.2

HTRamsey commented 3 months ago

@tony-josi-aws to your first point, the value would be in making it obvious to users why it is not compiling, instead of getting more questions on the forum. To the second point, the check is currently for v8.1, not v8.2.