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

[component ]add INIT_XXX_EXPORT_ORDER macro #5194

Closed
wants to merge 1 commit into from

Conversation

dongly
Copy link
Contributor

@dongly dongly commented Oct 15, 2021

拉取/合并请求描述:(PR description)

[

INIT_APP_EXPORT_ORDER(app_init_func1, 20);    /* 顺序为 0 ~ 99 */
INIT_APP_EXPORT(app_init_func2);              /* 默认顺序为 49 */

]

以下的内容不应该在提交PR时的message修改,修改下述message,PR会被直接关闭。请在提交PR后,浏览器查看PR并对以下检查项逐项check,没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 本拉取/合并请求代码是高质量的 Code in this PR is of high quality
  • 本拉取/合并使用formatting等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2021

CLA assistant check
All committers have signed the CLA.

@liukangcc liukangcc added the +1 Agree +1 label Oct 21, 2021
@BernardXiong BernardXiong added the -1 No vote label Oct 24, 2021
@BernardXiong
Copy link
Member

没这么随意加代码,加到99的吧

@mysterywolf
Copy link
Member

希望在4.0.5能有这个功能,同级初始化顺序不可控,一直被吐槽😢

@dongly
Copy link
Contributor Author

dongly commented Oct 25, 2021

没这么随意加代码,加到99的吧

范围可以改,如: 0 ~ 15,0 ~ 20,甚至是0~9,都可以.
代码有点丑,想了很多方法,但在不大幅改变代码,使用宏的话,实在想不出其他方法

@dongly dongly closed this Nov 4, 2021
@chenyingchun0312
Copy link
Contributor

这个为啥关闭呀,感觉这种有序初始化的方式,还挺需要的呢

@mysterywolf
Copy link
Member

先别关,这个一直是个痛点。

@mysterywolf mysterywolf reopened this Nov 8, 2021
@dongly
Copy link
Contributor Author

dongly commented Nov 8, 2021

找不到优雅的写法,也可将此代码独立一个文件

Comment on lines +261 to +271
/* initialization export order */
#define _INIT_ORDER_0 "00"
#define _INIT_ORDER_1 "01"
#define _INIT_ORDER_2 "02"
#define _INIT_ORDER_3 "03"
#define _INIT_ORDER_4 "04"
#define _INIT_ORDER_5 "05"
#define _INIT_ORDER_6 "06"
#define _INIT_ORDER_7 "07"
#define _INIT_ORDER_8 "08"
#define _INIT_ORDER_9 "09"
Copy link
Contributor

Choose a reason for hiding this comment

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

定义这么多宏定义实在意义不大, 使用的时候直接用字符串就可以了, 例如:

INIT_PREV_EXPORT_ORDER(fn, "3")

至于 default, 个人认为默认0就可以了

#define INIT_ORDER_DEFAULT "00"

或者这样定义, 就不用加双引号:

#define INIT_DEVICE_EXPORT_ORDER(fn, order) INIT_EXPORT(fn, "3." #order)

一点个人看法, 仅供参考,谢谢!

Copy link
Contributor Author

@dongly dongly Nov 15, 2021

Choose a reason for hiding this comment

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

  • 定义这么宏就是为了避免出现 10 高于 2 的问题
  • 若不定义这些宏,就按你的方案,这样 order 就是开放的。只能人为约定,不能写0 ~ 9,只能写00 ~ 09。也可是其他字符串。这样有潜在风险。(这种方案可授受?)
  • 至于 default 定义在中间值,就是为了可在默认 INIT_XXX_EXPORT(fn) 的之前或之后运行

@dongly
Copy link
Contributor Author

dongly commented Nov 15, 2021

这种方案如何?

  • 用法:
INIT_BOARD_EXPORT(fn);           /* 默认 */ 
INIT_BOARD_EXPORT_BEFORE(fn,8);  /* 在默认之前 */
INIT_BOARD_EXPORT_AFTER(fn,2);   /* 在默认之后 */

/* 自定义顺序(10~99) */
#define _INIT_ORDER_88  "88"
INIT_BOARD_EXPORT_AFTER(fn,88); 
  • 代码:
#define _INIT_ORDER_0  "00"
#define _INIT_ORDER_1  "01"
#define _INIT_ORDER_2  "02"
#define _INIT_ORDER_3  "03"
#define _INIT_ORDER_4  "04"
#define _INIT_ORDER_5  "05"
#define _INIT_ORDER_6  "06"
#define _INIT_ORDER_7  "07"
#define _INIT_ORDER_8  "08"
#define _INIT_ORDER_9  "09"

#define _INIT_ORDER(order)  _INIT_ORDER_##order

/* board init routines will be called in board_init() function */
#define INIT_BOARD_EXPORT_BEFORE(fn, order) INIT_EXPORT(fn, "1.0" _INIT_ORDER(order))
#define INIT_BOARD_EXPORT(fn)              INIT_EXPORT(fn, "1.100")
#define INIT_BOARD_EXPORT_AFTER(fn, order) INIT_EXPORT(fn, "1.2" _INIT_ORDER(order))

@mysterywolf mysterywolf removed the +1 Agree +1 label Nov 15, 2021
@thewon86
Copy link
Contributor

奇怪,知道外设初始化过程的依赖关系,为什么就不可以把两个有依赖关系的外设初始化过程放一起?在同一个初始化函数里进行,保证执行过程的正确顺序。
如果增加启动顺序编号数量,加多少才够?
同一阶段的初始化先后顺序,这是个应用紧密相关问题,是根据应用的需求而定的,不应该是实现的人需要注意的问题吗?

@dongly
Copy link
Contributor Author

dongly commented Nov 30, 2021

如果这样说,INIT_XXX_EXPORT()都没有存在的必要了,回归到早期的一个大一统的启动函数

@thewon86
Copy link
Contributor

thewon86 commented Dec 1, 2021

一个明确的,有限的几个启动阶段,和一个不确定的,没依据的启动编号。两者选哪个?

linux 已经放弃了 00-99 的启动编号顺序的方式了,大多发行版改用 systemd 设置启动,不是没有根据的。
两个要点,一个是设计几个启动阶段,每一个阶段都有明确定义和使用规范;另一个要点是,每一个阶段,不同子项之间的依赖关系由开发者维护。

我们应该做的可能是出一份使用说明和注意事项,而不是把启动阶段划分扩展到 100 1000 个

@dongly
Copy link
Contributor Author

dongly commented Dec 2, 2021

实现一个 systemd ? 好像挺复杂的.
与顺序有关的就有Reuires,Wants,After,Before,Binds of,Part Of, OnFailure,Conflicts.
简单实现一个Before ,After?

@thewon86
Copy link
Contributor

thewon86 commented Dec 3, 2021

实现一个 systemd ? 好像挺复杂的. 与顺序有关的就有Reuires,Wants,After,Before,Binds of,Part Of, OnFailure,Conflicts. 简单实现一个Before ,After?

没人说在 rtt 上实现 systemd 保持现状就好。用增加编号的办法解决依赖问题,不怎么高明,

@BernardXiong
Copy link
Member

实现一套 systemd ,似乎是个好的方式 😄

@dongly dongly deleted the INIT_EXPORT_ORDER branch March 31, 2023 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-1 No vote
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants