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

Closes #204 - Add WebSerial support. #205

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patrykwegrzyn
Copy link

Hi @colinbdclark i'm not familiar with testing framework you are using so this may need some work on your end.
Please review and let me know if is anything else i can do to help.
Best regards
Pat

@colinbdclark
Copy link
Owner

Hi @patrykwegrzyn, thanks so much for this PR. I'm swamped at the moment but will definitely take a close look and review this as soon as I can.

It looks like your editor maybe has automatically reformatted a number of files that you probably didn't mean to change. Can you make sure any white space changes etc. are omitted from your PR so that it's easier to review what has actually changed? Thanks so much!

@patrykwegrzyn
Copy link
Author

Hi @colinbdclark can you let me know what settings you are using ? im using prettier with standard settings i may be able to and do it but it may be painful.
As alternative I can make comments for you to highlight to what i have changed, i only added 1 line to Gruntfile , modified browser test and added osc-web-serialport.js . I did run build tho , probably that's why it looks like a lot stuff was changed.

oscWeb: [
"src/osc-transports.js",
"src/platforms/osc-websocket-client.js",
"src/platforms/osc-web-serialport.js",
Copy link
Author

Choose a reason for hiding this comment

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

I only changed this in a Grunt file

*/

/*global WebSerial, require*/
var osc = osc || require("../osc.js");
Copy link
Author

Choose a reason for hiding this comment

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

Followed your template and added new file to enable WebSerial support

}());
QUnit.test("Serial port support is not loaded", function () {
QUnit.expect(1);
QUnit.ok(osc.supportsSerial);
Copy link
Author

Choose a reason for hiding this comment

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

I modified this line to get tests to pass.

@ahfontaine
Copy link

Hi, just checking if this PR will ever get accepted? I am getting a lot of Webpack 5 issues due to polyfill not being supported anymore. I can revert but, would love to upgrade my project

@colinbdclark
Copy link
Owner

Hi @ahfontaine, I am in the midst of a big career transition that has been occupying my time recently, but the good news is that I anticipate having more time to focus on my open source projects soon. I'm sorry for the delay with this, and do have the intention of reviewing and merging when life permits.

@coder0107git
Copy link

@colinbdclark Any updates on this?

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