Skip to content

Sched note wdog#13901

Merged
xiaoxiang781216 merged 3 commits into
apache:masterfrom
XuNeo:sched-note-wdog
Oct 9, 2024
Merged

Sched note wdog#13901
xiaoxiang781216 merged 3 commits into
apache:masterfrom
XuNeo:sched-note-wdog

Conversation

@XuNeo

@XuNeo XuNeo commented Oct 8, 2024

Copy link
Copy Markdown
Contributor

Summary

  1. Add note support for wdog module.
  2. Update systemview correspondingly, using timer enter/exit.

Impact

No.

Testing

Tested with stm32f429i-disco board.

Configure

tools/configure.sh stm32f429i-disco:systemview

echo 123 > /dev/ttyR0 will trigger some wdog usage, so you can see the note is recorded.

sysview-wdog

@github-actions github-actions Bot added Area: Board support Board support issues Area: OS Components OS Components issues Size: M The size of the change in this PR is medium labels Oct 8, 2024
@nuttxpr

nuttxpr commented Oct 8, 2024

Copy link
Copy Markdown

[Experimental Bot, please feedback here]

The PR summary is OK, but lacks details on the exact changes made. The impact assessment is incomplete. Here's a breakdown of what needs improvement:

Summary - Needs Improvement

  • More Detail: Instead of just saying "Add note support," specify what kind of notes are added, how they are triggered, and what information they provide. For example: "Add systemview notes to track watchdog feeding, timeout events, and configuration changes."
  • Code Location: Mention the specific files or modules modified (e.g., "Changes made to the wdog driver and the SystemView integration layer").

Impact - Incomplete

  • New Feature: Explicitly state that this adds a new feature (systemview watchdog visualization).
  • User Impact: While the core functionality doesn't change, users utilizing SystemView for debugging will now have additional insights into the watchdog. This should be mentioned.
  • Build Impact: Does enabling this feature require any specific build options or configurations?
  • Documentation: Does the PR include documentation updates to explain the new notes and how to use them effectively?

Testing - Insufficient

  • Host Details: Provide the operating system and compiler version used for building.
  • Target Specificity: Mention the NuttX configuration used on your stm32f429i-disco (e.g., stm32f429i-disco:nsh).
  • Logs Missing: The provided example shows a screenshot, but the actual testing logs (both before and after) are absent. Include relevant log snippets demonstrating the changes in SystemView output.

In summary, the PR needs more detail in the summary and impact sections, along with complete testing information to meet the NuttX requirements.

Comment thread include/nuttx/sched_note.h Outdated
XuNeo and others added 3 commits October 8, 2024 17:56
Signed-off-by: xuxingliang <xuxingliang@xiaomi.com>
Signed-off-by: xuxingliang <xuxingliang@xiaomi.com>
Make it easy to access for beginners, using the default serial as console.
Enable wdog sched note.

Signed-off-by: Neo Xu <neo.xu1990@gmail.com>

@anchao anchao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@xiaoxiang781216 xiaoxiang781216 merged commit 066cd79 into apache:master Oct 9, 2024
@XuNeo XuNeo deleted the sched-note-wdog branch October 9, 2024 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Board support Board support issues Area: OS Components OS Components issues Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants