feat: use Arduino CLI 1.0.4 #2457
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
per1234
added
type: enhancement
giacomocusinato
force-pushed
the
use-cli-1.0.0
branch
from
8f03dee to
540d848
Compare
giacomocusinato
force-pushed
the
use-cli-1.0.0
branch
from
540d848 to
24296c5
Compare
giacomocusinato
force-pushed
the
use-cli-1.0.0
branch
from
24296c5 to
7ffd748
Compare
The npm package previously used (`protoc`) is still lacking apple arm32 support, see YePpHa/node-protoc#10
giacomocusinato
force-pushed
the
use-cli-1.0.0
branch
2 times, most recently
from
d396032 to
aca5ece
Compare
dankeboy36
reviewed
Jul 15, 2024
| @@ -182,7 +182,11 @@ export class ConfigServiceImpl | |||
| }); | |||
| const model = (yaml.safeLoad(content) || {}) as DefaultCliConfig; | |||
| this.logger.info(`Loaded CLI configuration: ${JSON.stringify(model)}`); | |||
| if (model.directories.data && model.directories.user) { | |||
| if ( | |||
| model.directories && | |||
There was a problem hiding this comment.
If directories is optional, I propose reflecting it on the type:
There was a problem hiding this comment.
I think directories can be undefined only on the initial parse of the current config.
We can manipolate the loaded config object to assure the directories are alway defined object returned by loadCliConfig is always of type DefaultCliConfig.
That's the current logic:
- Load current cli config as type
CliConfig(optionaldirectories). - Check
directoriesvalue:- If defined, return config as
DefaultCliConfigtype - If undefined, call
getFallbackCliConfigto get default directories, merge them to current config and return the object asDefaultCliConfig
- If defined, return config as
Comment on lines
224
to
229
| const rawJson = await spawnCommand(cliPath, ['config', 'dump', '--json']); | ||
| const config = JSON.parse(rawJson); | ||
|
|
||
| // Since CLI 1.0, the command `config dump` only returns user-modified values and not default ones. | ||
| // directories.user and directories.data are required by IDE2 so we get the default value explicitly. | ||
| const directoriesRaw = await spawnCommand(cliPath, [ |
There was a problem hiding this comment.
The CLI executions do not depend on each other: they can spawn parallel with Promise.all()
Comment on lines
321
to
316
| const client = createArduinoCoreServiceClient({ port }); | ||
| const req = new SettingsSetValueRequest(); |
There was a problem hiding this comment.
Is this a new CLI behavior? I see a client is created before setting each value. Can it be moved outside of the loop?
There was a problem hiding this comment.
Yes CLI config API's were refactored and they now only exposed methods for retrieving/setting single values, instead of the previous "merge" approach.
I agree the client should be created one time outside the loop. Thanks for the review! 👍
|
|
||
| const cliConfigUri = await this.getCliConfigFileUri(); | ||
| const cliConfigPath = FileUri.fsPath(cliConfigUri); | ||
| fs.writeFile(cliConfigPath, configRaw, { encoding: 'utf-8' }); |
There was a problem hiding this comment.
missing await or return
giacomocusinato
force-pushed
the
use-cli-1.0.0
branch
2 times, most recently
from
d78e917 to
15e2a19
Compare
giacomocusinato
changed the title
giacomocusinato
force-pushed
the
use-cli-1.0.0
branch
from
15e2a19 to
92f518d
Compare
giacomocusinato
changed the title
Arduino deprecated platforms should have more priority then other deprecated ones
davegarthsimpson
approved these changes
Sep 5, 2024
dankeboy36
added a commit
to dankeboy36/ardunno-cli-gen
that referenced
this pull request
Nov 17, 2024
Ref: arduino/arduino-ide#2457 Signed-off-by: dankeboy36 <[email protected]>
dankeboy36
added a commit
to dankeboy36/ardunno-cli-gen
that referenced
this pull request
Nov 17, 2024
* fix: patch for missing `google` proto files in CLI (arduino/arduino-cli#2755) * chore: replace `protoc` with `@pingghost/protoc` (arduino/arduino-ide#2457) * chore(ci): build on Node.js 20.x * fix(doc): refresh docs, update protoc version * chore: update formatter + fix example command --------- Signed-off-by: dankeboy36 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Use Arduino CLI
1.0.0APIs in IDE2.Change description
Update CLI version to
1.0.0, bind new APIsOther information
Reviewer checklist