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

Enhancement(web-serial): Enable Serial Options During Port Initialization #115

Merged

Conversation

RushikeshPatange
Copy link
Contributor

What Does This PR Do ?

@brianignacio5 Could you please take a look into this ?

@balloob
Copy link
Contributor

balloob commented Oct 30, 2023

Since you're already able to pass in a port, is this PR still necessary? You can create/configure your port accordingly before passing it to esptool.

@adwait-esp
Copy link
Collaborator

Since you're already able to pass in a port, is this PR still necessary? You can create/configure your port accordingly before passing it to esptool.

With reference to the Weberial API Documentation - the SerialPort open method supports SerialOptions dictionary. https://wicg.github.io/serial/#serialoptions-dictionary.

We currently support passing only baudrate, where as other options would come handy as well. This PR still maintains baudrate as primary parameter (to preserve compatibility and not introduce any breaking changes). However we can phase it out later and make use of serialoptions dict to be passed instead in future.

src/esploader.ts Outdated Show resolved Hide resolved
src/webserial.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@brianignacio5 brianignacio5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I think we could probably remove transport from LoaderOptions and add baud rate as required in SerialOptions to simplify later. For a next major or minor version release.

@adwait-esp adwait-esp merged commit 2864a73 into espressif:main Nov 1, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

4 participants