search

rust-embedded / cortex-m-quickstart

Template to develop bare metal applications for Cortex-M microcontrollers
782 stars 164 forks source link

VS Code Support #78

Closed rubberduck203 closed 4 years ago

rubberduck203 commented 4 years ago

I'm not sure if the project is interested in this, but I couldn't figure out any way to keep my VS Code template separate from the cortex-m-quickstart template.

This patchset adds VS Code debug configurations/examples for both QEMU and OpenOCD (lm3s6965evb and stm32f303vct6 respectively). I couldn't find an SVD file for the Texas Instruments controller, so there's no in editor peripheral viewer for that one.

rust-highfive commented 4 years ago

r? @korken89

(rust_highfive has picked a reviewer for you, use r? to override)

thejpster commented 4 years ago

Hmm, we might need a vote on whether to include this or if we want it as cortex-m-quickstart-vscode or somesuch.

Either way, could you add a # Microsoft VSCode section to the README and explain what the tasks do and what the prequisites are? I gave it a go (not knowing much about VSCode tasks) and I found the setup a bit fiddly.

rubberduck203 commented 4 years ago

I’m open to maintaining it myself if anyone has ideas on how to separate it, while still making it easy to get up and running. Otherwise, I’m going to end up maintaining a fork, which I’d like to avoid.

I’m obviously happy to add a section to the readme as well. Will push up a commit when I get a moment.

rubberduck203 commented 4 years ago

@thejpster I am curious what you found to be fiddly, so that I can document better. I noticed that on an initial cargo generate I had to wait for the editor to stop processing before I launched a debug session, but that’s the only thing I noticed. I’m curious what trouble you had.

rubberduck203 commented 4 years ago

I added documentation.

I think I found the "fiddly"-ness you mentioned. You have to trigger the RLS by opening a *.rs file or the build task won't be found. It makes sense, but is definitely less than ideal. I'm trying to figure out if it's a bug in the extension, RLS, or VS Code itself so I can report the behavior to the right place.

therealprof commented 4 years ago

Otherwise, I’m going to end up maintaining a fork, which I’d like to avoid.

The templates are kind of ment to be plentyful to cater for a lot of different needs. I agree it would be supernice to have a good template for vscode but I also have to agree with @thejpster that this might be a good candidate for a new crate which we might even host right next to the simple template.

I must admit I'm not using any of those and neither am I sure what status they're in and whether people (still) find them useful in general.

rubberduck203 commented 4 years ago

The problem with making it a different template is I can’t cargo generate two projects into the same space, as far as I’m aware. The VS Code part of the template does rely on the {{project-name}}.

I’m happy to maintain the feature, whether here or in a different repo, but I don’t know how it could possibly be split into a separate repo. If anyone has an idea on how to accomplish that, I’m happy to go down that path.

I personally don’t see why adding basic editor configs wouldn’t be in scope for a QuickStart template. As a user of the template, I like the convenience of cargo generate & go. That includes the convenience of a preconfigured editor IMO. I’d love to see others add configs for CLion & Vim.

therealprof commented 4 years ago

The problem with making it a different template is I can’t cargo generate two projects into the same space, as far as I’m aware. The VS Code part of the template does rely on the {{project-name}}.

So? Why would you need more than one?

I’m happy to maintain the feature, whether here or in a different repo, but I don’t know how it could possibly be split into a separate repo. If anyone has an idea on how to accomplish that, I’m happy to go down that path.

Would need to check the implementation but it should be somehow possible to split out the common parts into a seperate crate/module and just keep the "on-top" details to the crate called into by generate.

I personally don’t see why adding basic editor configs wouldn’t be in scope for a QuickStart template. As a user of the template, I like the convenience of cargo generate & go. That includes the convenience of a preconfigured editor IMO. I’d love to see others add configs for CLion & Vim.

So whenever you generate a quickstart project you'd automatically get all possible IDE configurations (potentially even outdated/incorrect), too? Sounds like a hot mess to me.

rubberduck203 commented 4 years ago

I’d also like to call out that I’ve seen several people claim that lack of editor support as one of the reasons they’re not adopting Rust for their embedded development. One such example here, although, I’d say the awkwardness of interrupt handling (disabling ALL interrupts) and lack of a safety certified compiler are the bigger blockers for many.

https://users.rust-lang.org/t/rust-2020-growth/34956/102

This could be one small step toward showing folks that there is indeed pretty good editor support, for at least one editor. If we could then add configs for CLion & Vim, we could cover the preferences of the vast majority of devs.

rubberduck203 commented 4 years ago

So? Why would you need more than one?

I don’t. I want a single template. That’s why I added it here.

Would need to check the implementation but it should be somehow possible to split out the common parts into a seperate crate/module and just keep the "on-top" details to the crate called into by generate.

I’m not terribly familiar with cargo generate, but I have the impression that it’s just taking text in a repo and replacing some tokens. I’m happy to be proved wrong here.

So whenever you generate a quickstart project you'd automatically get all possible IDE configurations (potentially even outdated/incorrect), too? Sounds like a hot mess to me.

Does it? It honestly doesn’t sound crazy to me to support a handful of the editors we know people are actually using to do CortexM dev with Rust. It’s all about reducing friction to gain further adoption. You also have a volunteer to help maintain the VS Code config.

therealprof commented 4 years ago

I don’t. I want a single template. That’s why I added it here.

That's not what I meant. If you have environment specific templates, why would you want more than one at a time?

Does it? It honestly doesn’t sound crazy to me to support a handful of the editors we know people are actually using to do CortexM dev with Rust. It’s all about reducing friction to gain further adoption. You also have a volunteer to help maintain the VS Code config.

The crazy part here is throwing them all in one pot and supporting them at the same time. If you're a VSCode user, why would you want Emacs, Vim, CLion and Eclipse support in your generated crate?

rubberduck203 commented 4 years ago

You’ve never worked on a project where different people worked with different editors? Having the extra configs doesn’t matter to a developer. I’m not using that editor, so I simply ignore their existence.

I’m just trying to make the experience of getting up and running as frictionless as possible. I thought that would be a primary goal of this project.

rubberduck203 commented 4 years ago

Listen guys, I used to maintain a successful project. 90% of my effort went into telling people a feature was out of scope. So, I understand if you decide this is out of scope. I'll maintain a fork if I have to, but I'd rather not. I really do think this is a good thing to add to the project. If I didn't, I wouldn't have taken the time to submit the PR.

therealprof commented 4 years ago

You’ve never worked on a project where different people worked with different editors?

Sure, but everyone is enabling their own setup. People do not generally allow support for custom setups in their checked in code.

Having the extra configs doesn’t matter to a developer.

It does matter. It clutters up the source code and adds expectations to a project that someone will actively manage the setup. Both the upkeep and the ensuring that available configurations work and are of equal quality are not goals of this project. Those templates are meant as a one-shot bootstrap method.

therealprof commented 4 years ago

@rubberduck203 We're looking into options here. The idea of having automatic VS Code setup is very agreeable and I already told you my ideas of how we could make this work as part of a embedded WG endorsed project.

If you prefer to fork and do your own thing that is fine of course.

rubberduck203 commented 4 years ago

I certainly don't prefer to fork. I'm cool with sticking around to make sure the vscode configs continue to work into the future, as I'm actively using them, but I don't want to take on the extra burden of keeping a fork up to date with upstream if I don't have to.

Unfortunately, cargo generate takes a git repository (and optional branch) in as a template and spits out a project. In order to generate a new project from the template that works out of the box, the vscode configs need to be inside of that same template.

https://github.com/rust-embedded/cortex-m-quickstart/pull/78/files#diff-3090e25fed17a6bd57433c6bbadc451aR17

You say you've given me your ideas on how to make this work, but all you've suggested is creating a new repository specifically for these configs, which won't actually work.

rubberduck203 commented 4 years ago

Does cargo generate work with git submodules?

therealprof commented 4 years ago

You say you've given me your ideas on how to make this work, but all you've suggested is creating a new repository specifically for these configs, which won't actually work.

I don't see why it wouldn't work.

I was kind of hoping we could add custom code to be executed but I just checked and it's really only templating stuff. :(

rubberduck203 commented 4 years ago

It looks like generate does support submodules. https://github.com/ashleygwilliams/cargo-generate/blob/1bb0fa8346942c8ed62660c2726222993d98f4e8/tests/integration/basics.rs#L448

Let me see what I can do.

rubberduck203 commented 4 years ago

Ok, so I've done some experimenting.

My first attempt was to have a repository that held the VS Code configs and submoduled in cortex-m-quickstart.

/
|-.vscode/
|  |- launch.json
|  |- tasks.json
|
|-cortex-m-quickstart files

This won't work because git forces a subdirectory and what you end up with is this.

/
|-.vscode/
|  |- launch.json
|  |- tasks.json
|
|-cortex-m-quickstart/
   |- src/
   |- README.md
   |- ...

This puts the quickstart files down one directory too far.

I then tried it the other way around, submoduling the editor config into cortex-m-quickstart. This works and creates the desired directory structure. A project generated from this template is identical to the one generated from this pull request.

However, I don't think it's a good idea to have a repository containing just the configs and using a submodule to pull it into the quickstart. You had expressed concern about cluttering the generated project with editor configs, this will continue to do that (even if I disagree that it's a real problem). On top of that, it actually becomes harder to maintain the quickstart if the .vscode/ directory is submoduled in. Yes, the configs could be maintained separately, but PRs to update the dependency here would only show the git sha change, not the actual changes being pulled into the quickstart. If I was the maintainer, I would prefer having the files directly maintained here over a submodule.

I'm earnestly open to suggestions here. I'm not sure what else to try that would result in a satisfactory solution for everyone.

TL;DR:

  1. The .vscode directory must be inside the root of the generated rust project in order to function properly.
  2. The .vscode configs rely on the {{project-name}} argument to work properly, so must be part of the template to get a smooth cargo generate experience.
  3. .vscode can be submoduled into cortex-m-quickstart, but cortex-m-quickstart can not be submoduled into a cortex-m-quickstart-vscode project.
rubberduck203 commented 4 years ago

I also would like to quickly address a comment I hadn't noticed earlier.

Those templates are meant as a one-shot bootstrap method.

So are these editor configurations. I fully expect users to either delete them outright, ignore them, or modify them for their own specific needs (mcu and frequency adjustments most likely).

thejpster commented 4 years ago

So after some reflection, and given the changes to the README, I think I'm now actually happy with including the .vscode folder in the quickstart template. I do have a couple more thoughts though:

On a semi-related note, it seems like it would be nice if there was an SVD server you could just hit with a part number (and perhaps a peripheral) to get register descriptions - saves us having to scrape them out of various IDEs and hide them in various Github repositories. A bit like the Windows debug symbol server.

Thank you for persevering with this one! We're nearly there :)

therealprof commented 4 years ago

@thejpster Thanks for chiming in with another opinion. I agree with your assessment and proposed changes.

nicholastmosher commented 4 years ago
  • Could we add a note to the README that says if you don't want VS Code support, just delete the .vscode folder (this assumes we've moved the SVD into this folder).

One thing I'd point out is that if .vscode is committed but you wanted to change or delete it, then you'll either

rubberduck203 commented 4 years ago
  • The SVD file seems to stick out. It's very chip specific, but most people won't know what it is or what it's for, so they'll just leave it there in their project thinking it's important. I wonder if we should move it into the .vscode folder, but also add a note to README explaining what it does and how to change it if you don't happen to be using that exact microcontroller.

I honestly considered not including it and just leaving instructions on how to obtain it, but you don't get a fully working editor without it. Moving it to the. .vscode directory makes a ton of sense to me.

  • Ditto for the instructions in the README - some guidance on how to modify it to support your specific chip would be useful.

Good idea.

  • Could we add a note to the README that says if you don't want VS Code support, just delete the .vscode folder (this assumes we've moved the SVD into this folder).

Absolutely. I think I'm also going to move the bulk of the VS Code docs to the .vscode folder and link to it from the main README. I'll put the note you're asking for in the main README.

  • Are JSON files allowed to have comments? The standard (and Github's renderer) say no, but I guess VS Code doesn't care so much?

Technically, no, JSON files are not allowed to have comments according to the spec, but whatever parser vscode uses parses them fine.

One thing I'd point out is that if .vscode is committed but you wanted to change or delete it, then you'll either

  • have git constantly nagging you about the untracked changes to .vscode/, or
  • you'd need to add that change as a commit, in which case it'd be easy to accidentally push it to some remote, and it'd be cumbersome to constantly rebase against new changes from upstream

The .vscode/ directory is ignored. I explicitly added only the tasks.json and launch.json files, so this won't be a problem. I also don't really expect people to be pulling changes from upstream after they've generated their project.

Also, FWIW, I stumbled across this Jetbrain's Rust survey the other day. It indicates that 42% of Rust devs are using VS Code, followed by Vim & CLion at 20% each, so I'm really happy we were able to come to a solution here. Thanks for bearing with me.

TODO:

Did I miss any action items?

thejpster commented 4 years ago

it'd be cumbersome to constantly rebase against new changes from upstream

Does anyone rebase against cortex-m-rt-quickstart though? If you run cargo generate it creates you a new git repo with no files added - you have to add the ones you want manually.

rubberduck203 commented 4 years ago

I pushed up a few more commits addressing the feedback. Ready for another review.

rubberduck203 commented 4 years ago

Quick summary of yesterday’s embedded WG meeting in regards to this PR:

rubberduck203 commented 4 years ago

Short term, how do you all feel about removing the SVD and just having instructions on how to obtain it? It may take me a while to work out the SVD pack thing.

perlindgren commented 4 years ago

@rubberduck203, great work.

I've been using VS Code for embedded development extensively over the last years. I'm teaching classes in embedded systems at Luleå University of Technology, where the students get to design and program embedded systems. Usually I have prepared ready made repositories so they can get stated, with pre-configured .vscode. (Using Cortex Debug).

Research wise we have been developing the RTFM framework for safe and efficient concurrency. It just struck me that besides a quickstart template for rt an rtfm quickstart could be very useful (with similar goals as the rt quickstart, with the .vscode support out of the box).

I have opened an issue at https://github.com/rtfm-rs/rfcs/issues/26 for anyone interested in further discussing that.

thejpster commented 4 years ago

Short term, how do you all feel about removing the SVD and just having instructions on how to obtain it? It may take me a while to work out the SVD pack thing.

I'm quite happy with that approach. Avoids files people don't need and any weird licensing issues.

rubberduck203 commented 4 years ago

@thejpster I removed the SVD (including removing it from the history). I think this is ready to go now.

thejpster commented 4 years ago

bors r+

bors[bot] commented 4 years ago

Configuration problem

bors.toml: not found

thejpster commented 4 years ago

Oh, we do this the old fashioned way...

rubberduck203 commented 4 years ago

I would like to thank everyone for sticking this one out with me! I’m very happy to see this merged.