search

probe-rs / vscode

VSCode debug extension for probe-rs. It uses the MS DAP protocol to communicate directly with the probe (via probe-rs), and supports basic command line debugging in addition to VSCode UI.
https://probe.rs/
Other
65 stars 6 forks source link

Refactor how `launch` and `attach` requests work. #12

Closed noppej closed 2 years ago

noppej commented 2 years ago

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.

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:

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

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 commented 2 years ago

@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 commented 2 years ago

Sounds good to me :+1:.

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

noppej commented 2 years ago

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

cc: @Yatekii

bors[bot] commented 2 years ago

:v: 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 commented 2 years ago

bors r+

bors[bot] commented 2 years ago

Build succeeded: