Skip to content

[kernel]add a macro RT_VERSION_CHECK #5850

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.

Sign up for GitHub

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

Merged
Guozhanxin merged 2 commits into from
Jul 4, 2022
Merged

[kernel]add a macro RT_VERSION_CHECK #5850

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da09080
add version check
thewon86 Apr 21, 2022
80b3ea4
Merge branch 'master' into version_check
thewon86 Jul 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions include/rtdef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
* 2022-01-07 Gabriel move some __on_rt_xxxxx_hook to dedicated c source files
* 2022-01-12 Meco Man remove RT_THREAD_BLOCK
* 2022-04-20 Meco Man change version number to v4.1.1
* 2022-04-21 THEWON add macro RT_VERSION_CHECK
* 2022-06-29 Meco Man add RT_USING_LIBC and standard libc headers
*/

Expand Down Expand Up @@ -71,8 +72,11 @@ extern "C" {
#define RT_REVISION 1 /**< revise version number */

/* RT-Thread version */
#define RTTHREAD_VERSION ((RT_VERSION * 10000) + \
(RT_SUBVERSION * 100) + RT_REVISION)
#define RTTHREAD_VERSION RT_VERSION_CHECK(RT_VERSION, RT_SUBVERSION, RT_REVISION)

/* e.g. #if (RTTHREAD_VERSION >= RT_VERSION_CHECK(4, 1, 0) */
#define RT_VERSION_CHECK(major, minor, revise) ((major * 10000) + \

@Guozhanxin Guozhanxin Apr 21, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

名字不太好，这个的作用更像是计算/生成一个数字，而不是检查

@mysterywolf mysterywolf Apr 21, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要是有检查 warning error的功能就更好了

@thewon86 thewon86 Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果要是有检查 warning error的功能就更好了

什么 waring error？

@mysterywolf mysterywolf Apr 21, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

就是定义一个宏定义，在文件底部或者文件的顶部，设置改文件最低允许的版本号，如果低于该版本号就会给出error或者waning，提示需要提升软件包版本或者降低内核版本。
我只是异想天开，仅供参考。

@thewon86 thewon86 Apr 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

就是定义一个宏定义，在文件底部或者文件的顶部，设置改文件最低允许的版本号，如果低于该版本号就会给出error或者waning，提示需要提升软件包版本或者降低内核版本。 我只是异想天开，仅供参考。

#if RTTHREAD_VERSION < RT_VERSION_CHECK(5, 0, 0)
#error "kernel version must >= 5.0.0"
#endif

放文件开头，就这样用

@thewon86 thewon86 Apr 26, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

/** Gives 1 if the x.y.z version is supported in the current version
 * Usage:
 *
 * - Require v6
 * #if LV_VERSION_CHECK(6,0,0)
 *   new_func_in_v6();
 * #endif
 *
 *
 * - Require at least v5.3
 * #if LV_VERSION_CHECK(5,3,0)
 *   new_feature_from_v5_3();
 * #endif
 *
 *
 * - Require v5.3.2 bugfixes
 * #if LV_VERSION_CHECK(5,3,2)
 *   bugfix_in_v5_3_2();
 * #endif
 *
 * */
#define LV_VERSION_CHECK(x,y,z) (x == LVGL_VERSION_MAJOR && (y < LVGL_VERSION_MINOR || (y == LVGL_VERSION_MINOR && z <= LVGL_VERSION_PATCH)))

仔细看它这个实现，看例子挺清爽，但是不仔细看实现，也不知道这个宏实现功能是啥，而且只实现了 >= 的逻辑运算。这样使用的时候也挺受限制的。
所以，lvgl 源码里搜索一下，发现大量下面用法

#if !(LVGL_VERSION_MAJOR == 6 && LVGL_VERSION_MINOR == 0)
    .subpx = LV_FONT_SUBPX_NONE,
#endif

”LV_VERSION_CHECK“对“非 6.0 版本” 这种情况表达无力，只能用 LVGL_VERSION_MAJOR LVGL_VERSION_MINOR 挨个比较。
还有

#if LV_VERSION_CHECK(7, 4, 0) || LVGL_VERSION_MAJOR >= 8
    .underline_position = -1,
    .underline_thickness = 1,
#endif

我没理解错的话，LV_VERSION_CHECK(7, 4, 0) 的意思就是 “大于等于 7.4.0 或者 7.4.0 以后的版本“。后面又混用了个 ”LVGL_VERSION_MAJOR >= 8“ 不知道当初改这行代码的人啥想法。

@Guozhanxin Guozhanxin Apr 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LV_VERSION_CHECK 的宏挺严谨的，里面包含了这三个版本值的 API 变更逻辑的。首先主版本号必须相同，然后，后面的版本号的变化 API 都是向前兼容的。所以这里才单独判断了下 || LVGL_VERSION_MAJOR >= 8

这里LV_VERSION_CHECK宏的名字就代表了他的功能，会把检查的逻辑写在宏内部。如果我们起名也要有check的话，就应该把check的逻辑实现在宏的内部。

@thewon86 thewon86 Apr 28, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

多定义一个宏，就是减少 MAJOR MINOR 几个宏的使用
这样混用只能说明那个 LV_VERSION_CHECK 定义的不足。这样混合使用还不如全用 MAJOR MINOR 几个判断来的清晰

不是说 LV_VERSION_CHECK 这样定义，RT_VERSION_CHECK 的含义就必须跟它是一样的。

@Guozhanxin Guozhanxin Jun 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最近有新的想法吗，希望在4.1.1上能把这个版本检查的功能合进去。

@Guozhanxin Guozhanxin Jun 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

过了段时间，我忽然感觉这种方式挺好的，如果只是站在使用者角度看，使用方法又灵活，又清晰。就是内部实现有点让人迷惑。如果没有更好的想法，这种方式我也可以接受

#if RTTHREAD_VERSION < RT_VERSION_CHECK(5, 0, 0)
#error "kernel version must >= 5.0.0"
#endif

(minor * 100) + revise)

/* RT-Thread basic data type definitions */
#ifndef RT_USING_ARCH_DATA_TYPE
Expand Down