Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Fix Arduino CLI: Upload command #1708

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

Conversation

Joshuabaker2
Copy link

@Joshuabaker2 Joshuabaker2 commented Jan 29, 2024

This PR is sort of two changes, hopefully that's okay.

The first change, an actual bugfix:

Fix the command that gets run when running Arduino CLI: Upload.

There was an error that would appear:

error: unknown flag: --build-path

Previously, the command used the pure "upload" command from arduino-cli, which meant it wasn't compiling then uploading, just uploading.

This didn't match the Arduino: Upload command, which does compile, and separately, it seems it was intended to do the upload command because there was a --build-path flag being passed, which is for the compile command.

I changed the code so that instead of an arduino-cli upload command, it now does an arduino-cli compile --upload command, which does two things:

  1. It makes it match the same functionality to the other Arduino: Upload command, and makes it so that it works with the flags that are passed in (those two facts make me relatively confident that was the intended purpose of this command in the first place)
  2. It fixes the bug where there was an unknown flag, as that flag was for the arduino-cli compile command.

The second change was to make the development experience better:

There seemed to be a few parts of the codebase where there was some assumptions made about the developer's environment. For example, the correct version of node and npm, a specific version of python installed, g++, etc. Trying to build the project on an M1 Macbook was very challenging, I spent a couple hours trying to get it all working properly.

In the end, I had to update a number of dependencies (the old ones were deprecated anyways and everything seems to work properly still) as well.

To tie it all together, I wrote a Dockerfile which sets up the development environment properly and builds the project.

Finally, I added two commands to the package.json "scripts" section:

  1. package which runs node ./build/package.js. This is just for convenience/discoverability since it took me a while to figure out how to even build the project.
  2. dockerbuild, which builds the docker file and then runs the package script, and outputs the resulting .vsix files in the ./out directory.

I also updated the README as appropriate.

Fixes:

#1686
#1646
#1319

@Joshuabaker2
Copy link
Author

@Joshuabaker2 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@Joshuabaker2
Copy link
Author

@gcampbell-msft I think you might be the maintainer of this package? Happy to change anything for this PR if needed.

@gcampbell-msft
Copy link
Contributor

@itodirel

@itodirel
Copy link
Member

itodirel commented Mar 6, 2024

@Joshuabaker2 Josh, sorry. We can't take this right now; we are only focusing on strictly blocking issues that have no workarounds for the Arduino extension.

@Joshuabaker2
Copy link
Author

Joshuabaker2 commented Mar 25, 2024

@itodirel would you be open to putting a notice at the top of the repo saying this package is unmaintained/not accepting PRs then? I spent a few hours getting this to work, it'd be nice for people to know to not spend time trying to contribute to the repo if there's no chance of the work being accepted.

@Borderliner
Copy link

Please merge this one. I just can't believe this issue has stayed open all this time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants