mcu-debug / rtos-views

RTOS views for microcontrollers
MIT License
24 stars 11 forks source link

Added ChibiOS RTOS Global Variable, Virtual Timers and Statistics, reworked createHmlHelp and refresh #34

Closed vrepetenko closed 1 year ago

haneefdm commented 1 year ago

It is not clear to me if this was handled but we expect more than one RTOS at the same time. This happens in multu-core debug where each core may be running an RTOS. So, the state info may have to be session specific. I would use the session name.

vrepetenko commented 1 year ago

It is not clear to me if this was handled but we expect more than one RTOS at the same time. This happens in multu-core debug where each core may be running an RTOS. So, the state info may have to be session specific. I would use the session name.

I think you mean this part:

            for (const rtosSession of this.sessionMap.values()) {
                promises.push(rtosSession.updateUIElementState(elementId, state));
            }
should be:

promises.push(currentSession.updateUIElementState(elementId, state));

But where currentSession can be found?
haneefdm commented 1 year ago

There is no concept of the current session. We need to pass the session-name to the new and all settings can be then stored as a combination of <session-name>-<rtos-name>-propname. So a chance would be specific to a session and its rtos.

I have to look in the code to figure out how to pass the session-name to the webview side

Also, there could be more than one session stopped. This is why there is no concept of a current session.

haneefdm commented 1 year ago

We can make each RTOS session info under a div whose element-id would be the session-name. Session names will be unique as VSCode will not allow dups (AFAIK).

So, if you extract the full path to your element, then that could be the key.

vrepetenko commented 1 year ago

We can make each RTOS session info under a div whose element-id would be the session-name. Session names will be unique as VSCode will not allow dups (AFAIK).

So, if you extract the full path to your element, then that could be the key.

Do you mean full Dom path? Can not get what problem we are trying to solve with this path... Now any RTOS implementation can use 'rtos-panels' or may not use it at all. Or xRTOS extension uses the same sessions list in different VSCODE windows, so if you open two VSCODE windows and run two different debug session there will be a problem with rtos-panels change event processing?

haneefdm commented 1 year ago

Okay, not sure we are on the same wavelength here. In a multi-core setup, we can have more than one debug session going at the same time. One session can use FreeRTOS and another can use ChibiOS. We put in a whole lot of effort to make this happen in a single instance of VSCode.

https://github.com/Marus/cortex-debug/wiki/Multi-core-debugging

You can also have multiple sessions independent of the cortex-debug method in the same instance of VSCode.

And, I am not talking about two different VSCode windows. That is a completely different topic.

vrepetenko commented 1 year ago

it is clear now, thanks for explanation! I will try to find a solution...

haneefdm commented 1 year ago

Hmmm. It appears that a build has failed. Looks like npm run compile failed. Something wrong with package.json or node_modules?

haneefdm commented 1 year ago

@PhilippHaefele when you get a chance, could you review this PR?

haneefdm commented 1 year ago

I would like to see the threads as the default (first) entry.

Agree. This would be the expectation

haneefdm commented 1 year ago

I would like to make a release soon. Quite a few things are batched up. I will merge this soon. I know @PhilippHaefele is busy for a couple of days but I would like him to finish his review.

Can you all update the CHANGELOG as needed if not already done?

PhilippHaefele commented 1 year ago

Only the default view discussion is open and after that being resolved, we're good to go from my side 👍

vrepetenko commented 1 year ago

Only the default view discussion is open and after that being resolved, we're good to go from my side 👍

check this out: https://github.com/mcu-debug/rtos-views/pull/34/commits/1bde86c7d1f42d245a82a857a03821540b9a9454

PhilippHaefele commented 1 year ago

Ups that somehow got hidden from me in the GitHub app 😄

@haneefdm So we're good to go

vrepetenko commented 1 year ago

I would like to make a release soon. Quite a few things are batched up. I will merge this soon. I know @PhilippHaefele is busy for a couple of days but I would like him to finish his review.

Can you all update the CHANGELOG as needed if not already done?

@haneefdm, should I update CHANGELOG with new PR?

haneefdm commented 1 year ago

I already made a release over the weekend. But, sure you can edit the CHANGELOG and update what I said. Add the PR# perhaps.

It is best to update the Changelog as we submit PRs.