search

Marus / cortex-debug

Visual Studio Code extension for enhancing debug capabilities for Cortex-M Microcontrollers
MIT License
1.01k stars 240 forks source link

ST-Link support and other contributions #2

Closed john5f35 closed 6 years ago

john5f35 commented 6 years ago

Hi, this is some great work that you are doing!

I'm wondering if you will be supporting ST-Link tools? I'm an assistant at a university and we will be using VSCode as an IDE for ARM development. We will be using PlatformIO community version for the project management, but would like to use this debugger plugin as opposed to a paid and less featured one from PlatformIO.

For simplicity of setting up, we'd like to use ST-Link tools which comes with PlatformIO tool chain as opposed to OpenOCD. So we are wondering if you will be interested in adding support or taking in merge request for this feature, and possibly some other features that we might need in the future.

Cheers again for the work!

Marus commented 6 years ago

Generally it isn’t too difficult to add at least basic support for another GDB server (The SWO decoding support can be more complex/not possible to support depending on the probe/gdb server combination).

I haven’t worked with the PlatformIO system, so I don’t know what ST-Link related tools they include. If you can find the details on the ST-Link tools that are included (is it possibly texane’s stlink tools? https://github.com/texane/stlink) it should be possible for me to add the basic support.

As for other features, feel free to make suggestions - will certainly consider them, or pull-requests (although I don’t know if I’d recommend starting with the code right away as I’m in the process of some substantial refactoring to clean things up and make it easier to expand, so may be better to wait for that to be completed)

john5f35 commented 6 years ago

I'm not very sure if it's Texane's one or the official one. It doesn't matter either way.

Refactoring sounds very good. In the mean time I have already written the support as a pull request #3.

john5f35 commented 6 years ago

Also, another feature request is about the register view. I think it's a great idea with what you are having now, but I think it might be better integrated if the register view is part of the variable request. A new scope called "Registers" and "Peripherals" can be created to group them.

The benefit would be better integration, and allowing the copying and setting of values.

If you are doing refactoring now it may be good to do. What do you think?

Marus commented 6 years ago

I'll take a look at the pull request later today.

Marus commented 6 years ago

I've merged in the pull requests; but I did make a few changes/additions from what you had:

I've published a .vsix file on GitHub (v0.1.9-pre); - please test this to make sure it is still working in your case (You'll need to manually install the .VSIX file). You will of course need to change "type": "stlink-gdb" to "type": "stutil-gdb" in your launch.json file.

Once you have confirmed it is still working I will publish the update to the visual studio marketplace and close this issue; it does seem to work with my install of the texane tools (which is what I expect PlatformIO is using), but best to confirm with your setup.

Also for the future, it is best to try to limit issues and pull requests to one topic. The stuff about the Registers and Peripherals views isn't really related to ST-Link/ST-Util support - so should be separate. Likewise the changes for the NPM building shouldn't have been part of the pull request (I've now put a better fix for that in place - that re-enables the pre-launch task but still has watching support); while I appreciate that you were trying to improve the tooling, it was unrelated and makes evaluating the changes more difficult.

Marus commented 6 years ago

Oh - and related to the register and peripheral views - it was an intentional choice to put them separate from the variables.

There is support for updating peripherals (register and field levels) - where it is permitted by the SVD file (right click on the register or field in the tree). I had this temporarily disabled as I was having some issues with it - these issues seem to be limited to under OpenOCD - so I've now re-enabled in that build for to other servers.

The core registers view does not currently support updates - can consider that for the future. Will also look into adding support for copying the current values from those views.

john5f35 commented 6 years ago

Cool, thanks! Sorry about some of the unrelated fixing. I will be more careful in the future.

I will test it on mine and let you know.

Marus commented 6 years ago

Oh not a big problem, especially something that small, just generally good practice. Let me know once you’ve tested and if it’s good I do the final publishing to the marketplace.

john5f35 commented 6 years ago

The pre-launch watch task is having problem on my machine. It builds and watches, but because the task doesn't quit, when I run the combined debug config the extension never launches. I'm not sure if it's because I'm using a different node/npm version?

The task config is quite old. The task config from a newly generated vscode project (yo code) looks like this:

// See https://go.microsoft.com/fwlink/?LinkId=733558
// for the documentation about the tasks.json format
{
    "version": "2.0.0",
    "tasks": [
        {
            "type": "npm",
            "script": "watch",
            "problemMatcher": "$tsc-watch",
            "isBackground": true,
            "presentation": {
                "reveal": "never"
            },
            "group": {
                "kind": "build",
                "isDefault": true
            }
        }
    ]
}

And changing the pre-launch task to be npm: watch in launch.json works for me.

Here is a patch file the contains the changes. Does it work for you? npm_task_patch.txt

Actually the problem with the above is that the task doesn't finish when the debug session is closed. Is there a better fix?

john5f35 commented 6 years ago

Apart from this everything works perfectly. :)

Marus commented 6 years ago

Ok. I’ll publish it to the marketplace. I already have a fix in place for the launch.json issue - the main fix is in the tasks.json file - they changed a parameter name at some point (November release I think) - was something like “isWatched” and now is “isBackground”. That may already be in the repo - if not I’ll push it up when I publish the update to the extension.

Thanks for your contributions.

Marus commented 6 years ago

This has been published to the marketplace.

FYI - I also implemented a "Copy Value" context menu command on registers and fields in the Cortex Peripherals and Cortex Registers views - just right click to access (won't show up for a Write Only field - as it doesn't make sense there).