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

Refactor how launch and attach requests work. #12

Merged
merged 8 commits into from
Oct 27, 2021
Merged

Refactor how launch and attach requests work. #12

merged 8 commits into from
Oct 27, 2021

Conversation

noppej
Copy link
Contributor

@noppej noppej commented Oct 14, 2021

This PR was motivated by discussions in #7, and will require rework of the equivalent functionality in probe-rs-debugger. It will involve a big code change in both repo's.

In the first iterations of this extension, the launch and attach terminology was interpreted to apply to the debug adapter process. In other words, "launch a new instance of the debug adapter", or "attach to a to an existing debug adapter process". - Both operations allowed flashing and resetting of the target process running on the probe.

  • Neither operations allowed the debug adapter session to last for more than one session of the target process on the probe.

The proposed change will reinterpret the launch and attach terminology to apply to the target process running on the probe. To help clarify the different behaviours, I will use the following terminology:

  • debug session: This refers to host side process, and ...
    • Starts when a user initiates either a launch or attach request, and ...
    • Ends when the user either initiates disconnect request, or chooses to Stop a host side debug session.
  • target session: This refers to the target side process, and ...
    • Starts when the target device boots and executes the firmware program, and ...
    • Ends when the target device processing halts as a result of a unrecoverable exception, or as part of a Reset request, or when the device looses power.

launch request type

  1. Start a debug session :
    i. Connect to the target device and executes a Reset request so that a new target session is started
    ii. Optionally flashes the target device firmware with the binary to be executed

attach request type

  1. Start a debug session :
    i. Connect to the target device, but do NOT issue `Reset, so that the existing target session is uninterrupted

Common behaviour to both launch and attach request types

  1. Support all standard debug behaviours, such as "Pause", "Breakpoints", RTT, etc.
  2. The Reset request will cause probe-rs, to restart the target device, and associated target session.
    i. This will honour the value of the halt_after_reset flag.
    ii. The debug session is not affected.
  3. The Disconnect request in the debug session will ...
    i. Instruct probe-rs to disconnect from the target device.
    ii. This implicitly ends the debug session.
    iii. The state of the target session is not affected.
    iv. There are some subtleties depending on launch vs attach as described in the MS DAP Specification for the Disconnect Request
  4. The Stop function (Terminate request) in the debug session will behave the same as the disconnect, except ...
    i. The debug session will send a stop request to halt the target session before disconnecting.
    ii. There are some subtleties depending on launch vs attach as described in the MS DAP Specification for the Terminate Request

Running probe-rs-debugger as a standalone for a VSCode debug session

In most cases, the VSCode debug extension will take care of automatically launching (and ending) the probe-rs-debugger executable to act as the debug adapter for probe-rs.

It is possible for the user to take control of this, by running probe-rs-debugger from the command line, with the following command

  • probe-rs-debugger debug --dap --port <any availably TCP/IP port number>

Any VSCode debug session can then attach to the specified on the server where probe-rs-debugger runs, and use the functionality above.

  1. Such a server debug session implies the following:
    i. In order for VSCode to know where to find the debug adapter, the user needs to add the appropriate TCP host IP address and port number to the launch.json configuration, by adding the option "server":"<host address>:<port number>" (e.g. "server":"127.0.0.1:50000")
    ii. Such a server debug session will put the management (start and stop) of the probe-rs-debugger process completely under control of the user.

Further reading:

The above proposal aims to be consistent with the process described in the VSCode debug docs

@noppej noppej linked an issue Oct 14, 2021 that may be closed by this pull request
@noppej
Copy link
Contributor Author

noppej commented Oct 15, 2021

@Tiwalun , @Yatekii , @huntc, @MabezDev

I would really appreciate if you could take a few minutes to review (and comment on) this proposal. It is a big chunk of work, and I'd like to get at least your alignment before I make the investment. I think it will be worth it to improve the user experience and ensure long term success/adoption of the debug extension.

Also, please feel free to invite others to comment if you think they can add to this discussion.

@Tiwalun
Copy link
Member

Tiwalun commented Oct 15, 2021

Sounds good to me 👍.

I wrote the original issue, and this is exactly the kind of change I had in mind.

@noppej noppej changed the title Refactor how launch and attach requests work. WIP: Refactor how launch and attach requests work. Oct 18, 2021
@noppej noppej requested a review from Tiwalun October 25, 2021 21:16
@noppej noppej changed the title WIP: Refactor how launch and attach requests work. Refactor how launch and attach requests work. Oct 25, 2021
@noppej noppej marked this pull request as ready for review October 25, 2021 21:17
@noppej
Copy link
Contributor Author

noppej commented Oct 25, 2021

@Tiwalun Please review this PR, and keep in mind that it has to be synched with both:

cc: @Yatekii

@noppej noppej requested a review from Yatekii October 25, 2021 21:22
Copy link
Member

@Yatekii Yatekii left a comment

Choose a reason for hiding this comment

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

Nice work :)

Just 2 smaller nits. Other than that it is looking great :)

bors d=noppej

src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Show resolved Hide resolved
@bors
Copy link
Contributor

bors bot commented Oct 27, 2021

✌️ noppej can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@noppej
Copy link
Contributor Author

noppej commented Oct 27, 2021

bors r+

bors bot added a commit to probe-rs/probe-rs that referenced this pull request Oct 27, 2021
854: Refactor how launch and attach requests work. r=noppej a=noppej

This is a refactoring of `probe-rs-debugger` code that is driven by changes documented in [VS Code extension PR # 12](probe-rs/vscode#12)

Co-authored-by: JackN <noppej@hotmail.com>
@bors
Copy link
Contributor

bors bot commented Oct 27, 2021

@bors bors bot merged commit b8b797f into master Oct 27, 2021
@bors bors bot deleted the Alpha0.3 branch October 27, 2021 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use of launch and attach requests is confusing
4 participants