noopkat / avrgirl-arduino

:girl: :pager: A NodeJS library for flashing compiled sketch files to Arduino microcontroller boards.
MIT License
506 stars 129 forks source link

Updating the browser-serialport dependency? #165

Closed sandeepmistry closed 6 years ago

sandeepmistry commented 6 years ago

Description

I'm using the latest version of avrgirl-arduino (in a Chrome app). Some upstream changes, particularly https://github.com/Pinoccio/js-stk500/commit/b61379ac954845b3fe2b660d23359a8138320cac is causing the uploads Arduino Mega 2560's to get stuck after the "reset complete" message.

The forked version of browser-serialport doesn't contain the isOpen() API, which was added in https://github.com/garrows/browser-serialport/commit/e288d629b6215acd9ece4c1d80fb99b7c7981060. There's two paths I can think of to move forward:

1) Cherry pick https://github.com/garrows/browser-serialport/commit/e288d629b6215acd9ece4c1d80fb99b7c7981060 into https://github.com/noopkat/browser-serialport#api-updates

2) Re-evaulate if latest browser-serialport in npm can be used as a dependency.

Let me know if there's another idea I missed :)

Note: this is not urgent to fix, I have a workaround that can be used in the short term (pinning stk500-v2 to 1.0.2).

Expected behaviour

Uploads to Mega2560 boards in a Chrome app environment to complete.

Actual behaviour

Uploads to Mega2560 boards in a Chrome app environment get stuck after the "reset complete" message.

Operating system and version

Chrome OS 68

Avrgirl Arduino version

2.2.11

Arduino Board being used

Arduino Mega2560

Log output, if available

connected

reset complete.

Step by step guide to reproducing the issue

  1. Using a Chrome OS app with the latest avrgirl-arduino, upload a hex file to a Arduino Mega 2560
noopkat commented 6 years ago

thanks for reporting this @sandeepmistry 💯

I think the path I'd like to go down is to see if avrgirl can start consuming the mainline of browser-serialport library again. The fork was used initially for compatibility with Serialport v4's breaking API changes.

If there are additional changes needed to do this (such as the commits from my fork), I can pull request mainline.

sandeepmistry commented 6 years ago

@noopkat great!

I pulled the latest browser-serial over, if we can get https://github.com/noopkat/browser-serialport/commit/a1cecbee1276bfe78b0491f8d13544c70859ff36 into upstream we are good to go. I see you already opened https://github.com/garrows/browser-serialport/pull/43 a while back though however.

Overall there's only been one code upstream that's not in your fork: https://github.com/garrows/browser-serialport/commit/e288d629b6215acd9ece4c1d80fb99b7c7981060

noopkat commented 6 years ago

@sandeepmistry I cherry-picked that commit into the api-updates branch of my fork. I also have released a new patch of avrgirl-arduino which pins to this specific latest commit, just for safety reasons to improve the potential stability should we change that branch again.

If you can confirm that this has unblocked you, we can close this issue. Thanks so much for your help on this, and as always, your patience! 😄

image

sandeepmistry commented 6 years ago

Hi @noopkat,

Thanks for looking at this! I can confirm the v2.2.13 release resolves this issue 😀.

noopkat commented 6 years ago

Great! We're also working on getting Browser Serialport better maintained and supported going forward. We'll get my fork and the mainstream back on track again. Thanks so much for your help!