eclipse-cdt-cloud / cdt-gdb-adapter

CDT GDB Debug Adapter
Eclipse Public License 2.0
28 stars 40 forks source link

Disconnect before exit for remote targets #273

Closed AdhamRagabMCHP closed 1 year ago

AdhamRagabMCHP commented 1 year ago

This changes fixes an issue whereby when debugging on a remote target, the exit sequence is not entirely correct. We should send a "disconnect" GDB command before "exit". This prevents a messages such as "Remote doesn't know how to detach" and "Cannot execute this command while the target is running" from appearing, as they can be somewhat misleading.

To recreate this:

To check that this works:

jonahgraham commented 1 year ago

@AdhamRagabMCHP We'll give it a few days to give others a chance to comment. Feel free to ping me next week to merge it if there has been no further comment.

AdhamRagabMCHP commented 1 year ago

Sounds good!

thegecko commented 1 year ago

lgtm

AdhamRagabMCHP commented 1 year ago

@AdhamRagabMCHP We'll give it a few days to give others a chance to comment. Feel free to ping me next week to merge it if there has been no further comment.

Hello Jonah! I'm not sure if this PR is good to merge now - we can wait an additional day or so, but just curious if now's a good time to merge.

sdirix commented 1 year ago

I did not test the PR but from my understanding it should work fine for the Blueprint use cases. Therefore I'm fine with merging :+1:

jonahgraham commented 1 year ago

Thanks @thegecko and @sdirix for weighing in.

jonahgraham commented 1 year ago

Thanks @AdhamRagabMCHP for the fix. I can do a new version now, or wait until #271 is done. Let me know if you want it now.

AdhamRagabMCHP commented 1 year ago

Thanks @AdhamRagabMCHP for the fix. I can do a new version now, or wait until #271 is done. Let me know if you want it now.

I think we'll wait until #271 is done - it makes sense to have all the changes in first.