Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

regression: allow monitor to not require to specify the board if the port cannot be identified. #2647

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

Merged
cmaglie merged 5 commits into arduino:master from cmaglie:monitor_defaults
Jun 24, 2024

Conversation

Copy link
Member

@cmaglie cmaglie commented Jun 19, 2024

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

If a port cannot be identified (i.e. can not be associated with a board) the current version of the CLI requires to specify a board with the -b flag. Previously this wasn't necessary and the monitor would sometimes fail to work because it may require special dtr / rts settings.

This PR now reverts the old behavior that is more convenient but also adds a warning to the user when the board can not be identified and the default settings may not work as expected.

What is the current behavior?

$ arduino-cli board list
Port Protocol Type Board Name FQBN Core
/dev/ttyS4 serial Serial Port Unknown
$ arduino-cli monitor -p /dev/ttyS4
Please specify an FQBN. The board on port /dev/ttyS4 with protocol serial can't be identified
$ arduino-cli monitor -p /dev/ttyS4 -b esp32:esp32:esp32
Connecting to /dev/ttyS4. Press CTRL-C to exit.
^C

What is the new behavior?

$ arduino-cli board list
Port Protocol Type Board Name FQBN Core
/dev/ttyS4 serial Serial Port Unknown
$ arduino-cli monitor -p /dev/ttyS4
Using generic monitor configuration.
WARNING: Your board may require different settings to work!
Monitor port settings:
 baudrate=9600
 bits=8
 dtr=on
 parity=none
 rts=on
 stop_bits=1
Connecting to /dev/ttyS4. Press CTRL-C to exit.
^C
$ arduino-cli monitor -p /dev/ttyS4 -b esp32:esp32:esp32
Using default monitor configuration for board: esp32:esp32:esp32
Monitor port settings:
 baudrate=9600
 bits=8
 dtr=off <-- notice the esp32 requires DTR=off
 parity=none
 rts=off <-- and also RTS=off
 stop_bits=1
Connecting to /dev/ttyS4. Press CTRL-C to exit.
^C

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

Fix #2638

brightproject reacted with thumbs up emoji
@cmaglie cmaglie self-assigned this Jun 19, 2024
@cmaglie cmaglie added type: enhancement Proposed improvement topic: CLI Related to the command line interface labels Jun 19, 2024
Copy link

codecov bot commented Jun 19, 2024
edited
Loading

Codecov Report

Attention: Patch coverage is 47.61905% with 22 lines in your changes missing coverage. Please review.

Project coverage is 70.23%. Comparing base (7d00b5b) to head (ca0101f).

Files Patch % Lines
internal/cli/monitor/monitor.go 52.94% 16 Missing ⚠️
internal/cli/arguments/port.go 33.33% 4 Missing ⚠️
internal/cli/arguments/fqbn.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@
## master #2647 +/- ##
==========================================
- Coverage 70.26% 70.23% -0.03% 
==========================================
 Files 222 222 
 Lines 21338 21347 +9 
==========================================
 Hits 14993 14993 
- Misses 5165 5175 +10 
+ Partials 1180 1179 -1 
Flag Coverage Δ
unit 70.23% <47.61%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@per1234 per1234 added the topic: code Related to content of the project itself label Jun 20, 2024
@cmaglie cmaglie merged commit 850f22a into arduino:master Jun 24, 2024
@cmaglie cmaglie deleted the monitor_defaults branch June 24, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@umbynos umbynos umbynos approved these changes

Reviewers whose approvals may not affect merge requirements
Labels
topic: CLI Related to the command line interface topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Schema not defined for key 'sketchbook_path' / monitor command now requires FQBN -b ...

AltStyle によって変換されたページ (->オリジナル) /