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

Add support for using native Yosys/nextpnr #6

Closed
DanielleHuisman opened this issue Dec 21, 2022 · 0 comments · Fixed by #14
Closed

Add support for using native Yosys/nextpnr #6

DanielleHuisman opened this issue Dec 21, 2022 · 0 comments · Fixed by #14
Assignees
Labels
enhancement New feature or request

Comments

@DanielleHuisman
Copy link
Member

DanielleHuisman commented Dec 21, 2022

Currently, WorkerTaskTerminal is somewhat WebAssembly worker specific. Based on some extension setting, the task terminal should either use a WebAssembly worker or a native worker (using the child_process Node.js module probably). Ideally, they share as much code as possible.

I think we have two options for implementing it:

  • Split it into two subclasses
    • abstract class WorkerTaskTerminal
    • class NativeWorkerTaskTerminal extends WorkerTaskTerminal
    • class WebAssemblyWorkerTaskTerminal extends WorkerTaskTerminal
  • Keep the single WorkerTaskTerminal class, but move WebAssembly/native code into separate functions. Those functions could be in separate files. The functions could even create a new class instance, so WebAssembly/native code can be separated that way (e.g. NativeWorker/ WebAssemblyWorker).

The first approach would mean we also need two classes for Yosys/nextpnr, so that's not ideal.

For the configuration, I think we should use the extension configuration instead of our project configuration. What worker to use depends on the user's environment, so it should not be included in the project configuration. The VS Code API provides a configuration section in the contribution points (docs).

Our first priority is implementing this feature, but in the future we can ensure that native code (e.g. child_process imports) never occurs in the web based environment (docs). This means the extension can still work on vscode.dev (even though it is currently broken, see #7).

@DanielleHuisman DanielleHuisman added the enhancement New feature or request label Dec 21, 2022
@DanielleHuisman DanielleHuisman changed the title Add config option for using native Yosys/nextpnr Add support for using native Yosys/nextpnr Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants