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

Request: Consider splitting out the SVD Viewer #736

Closed
thegecko opened this issue Sep 1, 2022 · 11 comments
Closed

Request: Consider splitting out the SVD Viewer #736

thegecko opened this issue Sep 1, 2022 · 11 comments

Comments

@thegecko
Copy link
Contributor

thegecko commented Sep 1, 2022

The SVD Viewer included in this extension is only driven using the Debug Adapter Protocol interface and therefore could be used with any debugger.

Please consider extracting this functionality into its own extension so it can used by any debugger and avoid extension duplication in the marketplace and promote composability in VS Code.

This could be done in a similar fashion to the Memory Viewer.

Perhaps there is a larger piece of architecture here where cortex-debug, memory-viewer and svd-viewer are exposed separately under a new GitHub organisation (https://github.com/cortex-debug?).

An extension pack could then be created which brings these extensions together and offer the ultimate composability and still allow users to install all tools together.

I'm happy to help with doing this and in fact already proved it is possible here:

https://github.com/thegecko/svd-viewer

This "fork" includes a lot of bug fixes for dealing with SVD files from CMSIS packs and has been built to also work in a browser environment along with browser-based debuggers, e.g.:

https://marketplace.visualstudio.com/items?itemName=Arm.embedded-debug

@haneefdm
Copy link
Collaborator

haneefdm commented Sep 1, 2022

I am the author of memview and I am the sole maintainer of cortex-debug.

I've been waiting for a good memory viewer and MS had a great promise with their Hex Editor which is now used by VSCode natively and is launched from Variables/Watch windows.

I want to split out RTOS viewer as well that I put into cortex-debug. It shouldn't even be part of cortex-debug, the way I wrote it. It can be split up in a couple of days. While it is in the same repo, it is bundled with cortex-debug and cortex-debug doesn't even know about it.

Back to your SVD question. I think the time has come for us to kill what we got and just support thegecko. I mean there should have been a reason why you did it. I am assuming it is you. Microsoft also has one as part of their Embedded Tools extension but it is not open source.

Now, the big one :-) I don't own cortex-debug. The owner is busy doing other stuff and I have been maintaining cortex-debug for the past 2 years at least. I will contact the author (I doubt he is reading any mail from here). I could start an org. if that is needed and if @Marus agrees. A change in the name or publisher name can cause confusion in the user-land so, we gotta be careful

One issue that I have is that every extension has a debug-tracker now. I requested Microsoft to provide more events so we don't need those. I have counted 6-7 trackers now. This adds to the overhead of debugging because all those trackers are looking at something but all traffic is visiting them That is a TON of traffic -- even if we ignore most of it. This is not good for the overall health of VSCode debug.

I can release an extension that is just a debug-tracker and provide services to all other extensions who need it. Other extensions can just use an API but cross extension API calls are slow. The have to use IPC because each extension is sandboxed. It should still perform okay.

Also, could you take a look at this one and see if you can direct the user to your extension? Is your extension published yet? I could not find it in the marketplace.

#731

I hate to ask for this, but could you do me a favor and look at this and vote for the issue on the MS Website if you agree.

#729 (comment)

@thegecko
Copy link
Contributor Author

thegecko commented Sep 1, 2022

I want to split out RTOS viewer as well that I put into cortex-debug. It shouldn't even be part of cortex-debug, the way I wrote it. It can be split up in a couple of days.

That sounds like a great idea

I think the time has come for us to kill what we got and just support thegecko.

I'm happy to move the extracted svd-viewer to wherever we want and can offer some support.

I mean there should have been a reason why you did it.

Yes, we had a need to use this with other debug adapters (and there were some issues to fix).

I could start an org. if that is needed and if @Marus agrees. A change in the name or publisher name can cause confusion in the user-land so, we gotta be careful

To that end I registered the cortex-debug organisation. Perhaps we can ask @Marus if he is happy for us to transfer the core debug adapter there and we can create repos for svd-viewer, memory-viewer and the rtos-viewer?

I can release an extension that is just a debug-tracker and provide services to all other extensions who need it.

This sounds like a sensible abstraction. The risk is that VS Code changes the API later. Perhaps this should be considered after splitting the functionality apart?

Also, could you take a look at this one and see if you can direct the user to your extension?

OK, I'll have a look

Is your extension published yet? I could not find it in the marketplace.

Not yet, I would rather we merged efforts than flood the marketplace with similar forks. Perhaps I can publish it once we have an org/publisher. A future cortex-debug release could then depend on it (or better yet, we create an extension pack).

I hate to ask for this, but could you do me a favor and look at this and vote for the issue on the MS Website if you agree.

The issue I created?

@haneefdm
Copy link
Collaborator

haneefdm commented Sep 1, 2022

The issue I created?

No, this one that I created...and it is pinned on the Issues page.

#729 (comment)

Microsft had a deadline for voting and it is very soon. I just saw that yesterday, which is why I am asking people to hurry if they agree.

@thegecko
Copy link
Contributor Author

thegecko commented Sep 1, 2022

Sorry, I only see a link to this:

microsoft/vscode#155597

@haneefdm
Copy link
Collaborator

haneefdm commented Sep 2, 2022

Apologies. I did not realize that Microsoft Issue was initially written by you. I joined in the middle as a person from Microsoft also wanted the same and he added me to the thread.

Right now, I have a hard time convincing them of anything. They just don't understand C/C++. They are in the TypeScript/Javascript/Python mold. I was the one who fought to add the memory and disassembly requests more than 2 years ago.

Btw, we hit the 20 votes. I scrolled all the up to see the vote count I saw your profile pic. I did not know about you until after I had joined that thread.

@haneefdm
Copy link
Collaborator

Did you know that there are about three extensions that exist just to serve SVD files and our users use them. We have to maintain compatibility with those users without changing those extensions hopefully.

image

@thegecko
Copy link
Contributor Author

Did you know that there are about three extensions that exist just to serve SVD files and our users use them

I did, happy to support this when you are ready

@thegecko
Copy link
Contributor Author

thegecko commented Oct 21, 2022

Did you know that there are about three extensions that exist just to serve SVD files and our users use them

Actually, this would be far easier to support SVD files loading directly from the CMSIS packs. This would open up support for around ~600 targets and remove the need for these extension packs.

It would also receive updates/fixes to the svd files from source without having to re-release them and maintain them manually.

Let me see if this is something we can enable...

@haneefdm
Copy link
Collaborator

Yes, that would be a welcome change to pick up SVD files from CMSIS packs.

Watch out for startup time. offline behavior, need for caching. And, please, before implementing lets discuss the design and plan.

I will get back to you here when I finish moving the SVD stuff into its own repo in a 100% compatible way.

@haneefdm
Copy link
Collaborator

Yes, that would be a welcome change to pick up SVD files from CMSIS packs.

Yet, we cannot break compatibility.

@haneefdm
Copy link
Collaborator

Done. Fixed in https://github.com/mcu-debug/peripheral-viewer

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

No branches or pull requests

2 participants