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

Documentation: Add usage of Wireshark for USB monitor #15630

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JianyuWang0623
Copy link
Contributor

Summary

Add usage of Wireshark for USB monitor.
image

Impact

Documentation

Testing

  1. Selftest passed as snapshot in "Summary"
  2. CI

@github-actions github-actions bot added Area: Documentation Improvements or additions to documentation Size: S The size of the change in this PR is small labels Jan 21, 2025
@nuttxpr
Copy link

nuttxpr commented Jan 21, 2025

[Experimental Bot, please feedback here]

No. The PR is missing critical information required by the NuttX guidelines.

Here's why:

  • Insufficient Summary: While the summary mentions what changed (Wireshark usage for USB monitor), it lacks crucial details:

    • Necessity: Why is this change needed? What problem does it solve or what improvement does it offer?
    • Functionality: Which part of the code is affected? (e.g., USB driver, networking stack, tools)
    • Mechanism: How does it work? How is Wireshark integrated? What specific changes were made?
    • Issue References: Are there any related NuttX or NuttX Apps issues this PR addresses?
  • Incomplete Impact Assessment: While "Documentation" is mentioned, other impact categories are not addressed. Even for Documentation, it's insufficient to just say "Documentation." Specify what documentation is updated and how. All other impact areas (user, build, hardware, security, compatibility) must be explicitly marked as NO or YES, and explained if YES.

  • Insufficient Testing Information:

    • Build Host Details: Missing operating system, CPU architecture, and compiler version used for the build host.
    • Target Details: Missing target architecture, board, and configuration details.
    • "CI" is not enough: While CI passing is good, it's not a replacement for providing specific test logs or results demonstrating the change's functionality before and after the modification. The provided screenshot doesn't show the before state.

The PR needs to provide the missing details to be considered complete according to the NuttX requirements.

@JianyuWang0623 JianyuWang0623 force-pushed the br_wjy_doc_USB_mon_wireshark_Linux_250121 branch from 39f5cfa to 142cb6b Compare January 21, 2025 14:21
@acassis
Copy link
Contributor

acassis commented Jan 21, 2025

@JianyuWang0623 please include an image from Wireshark showing it displaying the USB communication over the endpoints

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Documentation Improvements or additions to documentation Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants