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

Tools: fix idf.py monitor reset with hotkey with --no-reset arg (IDFGH-7375) #8955

Merged
merged 1 commit into from
May 16, 2022

Conversation

nonoo
Copy link
Contributor

@nonoo nonoo commented May 16, 2022

Previous --no-reset PR (#8788) was not merged completely. Without this patch, the hotkey reset in idf.py monitor (ctrl+t ctrl+r) does not work when --no-reset argument is used.

@espressif-bot espressif-bot added the Status: Opened Issue is new label May 16, 2022
@github-actions github-actions bot changed the title Tools: fix idf.py monitor reset with hotkey with --no-reset arg Tools: fix idf.py monitor reset with hotkey with --no-reset arg (IDFGH-7375) May 16, 2022
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

@dobairoland
Copy link
Collaborator

Hi @nonoo. Oh I see it now. I assumed this was a mistake. I'm afraid the same mistake could happen in the future.

Could you add this into the run() method in tools/idf_monitor_base/serial_reader.py instead? That would make it clearer.

Something like the following:

if not self.gdb_exit and self.reset:
...
elif not self.reset:
...

@nonoo
Copy link
Contributor Author

nonoo commented May 16, 2022

serial_reader.py's run() is not executed when a hotkey reset is requested. A hotkey reset request causes a CMD_RESET which is handled in serial_handler.py.

@dobairoland
Copy link
Collaborator

But the hotkey reset is not working for you because the DTR/RTS signals were not left at the right state after the port has been opened. Or am I missing something?

…needed

This lets use the reset hotkey (Ctrl+T Ctrl+R).

Tools: fix idf.py monitor reset with hotkey with --no-reset arg

Tools: Set idf.py monitor DTR to the default state when reset is not needed
@nonoo nonoo force-pushed the idf-monitor-no-reset branch from 7c09e94 to c063cbd Compare May 16, 2022 11:08
@dobairoland
Copy link
Collaborator

Thanks for the update!

@dobairoland
Copy link
Collaborator

sha=c063cbd7f7d0996fbaab3d97c8011f216576c8fc

@dobairoland dobairoland added the PR-Sync-Merge Pull request sync as merge commit label May 16, 2022
@espressif-bot espressif-bot added Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels May 16, 2022
@espressif-bot espressif-bot merged commit 31b7694 into espressif:master May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Sync-Merge Pull request sync as merge commit Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants