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.

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

Jump to bottom

[stm32][drivers][assert] 将STM32 HAL assert与RTT assert接口对接 #4486

Closed
wants to merge 1 commit into from
Closed

[stm32][drivers][assert] 将STM32 HAL assert与RTT assert接口对接 #4486

wants to merge 1 commit into from

Conversation

mysterywolf
Copy link
Member

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

[
参见issue：https://github.com//issues/2026
]

以下的内容不应该在提交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

@mysterywolf mysterywolf requested a review from whj4674672 March 19, 2021 06:10
@mysterywolf mysterywolf added the BSP: STM32 BSP related with ST/STM32 label Mar 19, 2021
@mysterywolf mysterywolf changed the title [stm32][drivers][assert] 将STM32 HAL assert与RTTassert接口接驳 [stm32][drivers][assert] 将STM32 HAL assert与RTT assert接口对接 Mar 19, 2021
@mysterywolf mysterywolf removed the request for review from whj4674672 March 19, 2021 06:21
whj4674672
whj4674672 reviewed Mar 20, 2021
View reviewed changes
bsp/stm32/libraries/HAL_Drivers/SConscript
@@ -111,6 +111,8 @@ path += [cwd + '/config']
if GetDepend('BSP_USING_ON_CHIP_FLASH'):
path += [cwd + '/drv_flash']

group = DefineGroup('Drivers', src, depend = [''], CPPPATH = path)
CPPDEFINES = ['USE_FULL_ASSERT']
Copy link
Contributor

@whj4674672 whj4674672 Mar 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

这个宏在 stm32xxxx_hal_conf.h 中有定义

/* #define USE_FULL_ASSERT    1U */

这个功能是否启动是不是应该取决于用户是否需要呢？ST 的这种设计，是可以通过 CUBEMX 来开启该功能的。

Copy link
Contributor

Choose a reason for hiding this comment

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

image

@mysterywolf mysterywolf deleted the stm32assert branch March 20, 2021 10:59
@mysterywolf mysterywolf restored the stm32assert branch March 25, 2021 11:19
@mysterywolf mysterywolf reopened this Mar 25, 2021
@mysterywolf
Copy link
Member Author

@Guozhanxin 郭老师看看这个有没有对接的必要
也有小伙伴在论坛上反应反应过这个问题
反正这个pr也挺难受的 还有几个bsp报错 目前这个pr是有问题的 是否应该对接 怎么对接可能还需要讨论一下
https://club.rt-thread.org/ask/question/430528.html

@mysterywolf mysterywolf added the discussion This PR/issue needs to be discussed later label Mar 25, 2021
@mysterywolf mysterywolf requested a review from Guozhanxin March 25, 2021 11:22
@Guozhanxin
Copy link
Member

直接这么改是不行的，会报警告

image

我能想到的方案就是，在 scons 脚本里，读一下当前bsp下的cubemx生成的文件，如：https://github.com/RT-Thread/rt-thread/blob/master/bsp/stm32/stm32f407-atk-explorer/board/CubeMX_Config/Inc/stm32f4xx_hal_conf.h

如果里面开启了 USE_FULL_ASSERT，就导出一个全局的宏 STM32_USE_FULL_ASSERT，然后我们就可以在驱动里实现 assert_failed 这个接口了。

@mysterywolf
Copy link
Member Author

好麻烦啊 要不就按照海靖说的 让用户自己在cubemx里边√ ？因为当年海靖提出这个问题的时候 #2026 bsp还是标准库，现在已经变成HAL库了 经过海靖提醒之后 我觉得貌似没有太大必要进行对接了。

@Guozhanxin
Copy link
Member

好麻烦啊 要不就按照海靖说的 让用户自己在cubemx里边√ ？因为当年海靖提出这个问题的时候 #2026 bsp还是标准库，现在已经变成HAL库了 经过海靖提醒之后 我觉得貌似没有太大必要进行对接了。

这个问题不是用户打不打勾的问题，而是，他打了勾我们却不知道他打没打勾。。。所以不知道什么时候需要定义 assert_failed 这个函数

@Guozhanxin
Copy link
Member

问题就是，他打了勾，却没有对接这个函数的话，会编译报错。
如果他没打勾，我们定义了这个函数，会报警告。

@mysterywolf mysterywolf marked this pull request as draft June 13, 2021 20:47
@mysterywolf mysterywolf closed this Jan 9, 2022
@mysterywolf mysterywolf deleted the stm32assert branch January 9, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whj4674672 whj4674672 whj4674672 left review comments

@Guozhanxin Guozhanxin Awaiting requested review from Guozhanxin

Assignees
No one assigned
Labels
BSP: STM32 BSP related with ST/STM32 discussion This PR/issue needs to be discussed later
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mysterywolf @Guozhanxin @whj4674672