-
Notifications
You must be signed in to change notification settings - Fork 133
IEP-1334: EIM Integration #1116
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
Changes from 41 commits
da7a35f
ff8e4dc
6390929
046c6d4
eb705a7
889e8ed
99781c9
de6b13d
a08fde6
bcbbfb1
86edcde
665c1ee
adc8643
71ddc37
85442c0
6840537
a70bd56
5b75467
7c5c165
ff02f73
7c8beb3
018ddb1
92f2fef
4fd767f
ac0375a
da08790
6b70e93
ad90d61
4b63c7b
325ba81
1501568
53d8f86
895598e
41f77b5
4187122
702c2b1
f45f1ca
249ada3
56855bf
a97d4fd
9319826
39fd98a
339ff8e
76f6fcd
7231f3c
3cc1389
b14efa7
1b414e8
7746a8f
e2f7f84
6ce4789
22138ed
36d2ecf
1e3319d
3d6852c
589456c
3b3640e
cec026f
098051e
e7143c1
3a46aa5
9754e26
4bb57ea
5035ae8
a855203
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,39 +10,34 @@ on: | |||||||||||||||||||||||||||||
| branches: [ master ] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||
| build: | ||||||||||||||||||||||||||||||
| build_linux: | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| runs-on: | ||||||||||||||||||||||||||||||
| - self-hosted | ||||||||||||||||||||||||||||||
| - eclipse | ||||||||||||||||||||||||||||||
| - BrnoUBU0004 | ||||||||||||||||||||||||||||||
| runs-on: [self-hosted, eclipse, BrnoUBU0004] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||
| - uses: actions/checkout@v2 | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| - name: Clone IDF Release From Github | ||||||||||||||||||||||||||||||
| uses: actions/checkout@v2 | ||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||
| repository: espressif/esp-idf | ||||||||||||||||||||||||||||||
| path: dependencies/idf-tools | ||||||||||||||||||||||||||||||
| submodules: 'true' | ||||||||||||||||||||||||||||||
| ref: release/v5.1 | ||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| - name: Set up Python | ||||||||||||||||||||||||||||||
| uses: actions/setup-python@v2 | ||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||
| python-version: '3.10' | ||||||||||||||||||||||||||||||
| python-version: '3.10' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
Comment on lines
+22
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Update The current setup uses an outdated version of the action. Update to v4 for the latest security improvements and features: - - uses: actions/checkout@v4
+ - uses: actions/checkout@v4
- name: Set up Python
- uses: actions/setup-python@v2
+ uses: actions/setup-python@v4
with:
python-version: '3.10'📝 Committable suggestion
Suggested change
🧰 Tools🪛 actionlint (1.7.4)21-21: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue (action) |
||||||||||||||||||||||||||||||
| - name: Install ESP-IDF via eim | ||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix YAML syntax error: Remove tab character. The step name contains a tab character which can cause YAML parsing issues. - - name: Install ESP-IDF via eim
+ - name: Install ESP-IDF via eim📝 Committable suggestion
Suggested change
🧰 Tools🪛 YAMLlint (1.35.1)[error] 25-25: syntax error: found character '\t' that cannot start any token (syntax) |
||||||||||||||||||||||||||||||
| uses: espressif/install-esp-idf-action@v1 | ||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||
| version: 'v5.3' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolve YAML Syntax Issue and Confirm ESP-IDF Action Security A tab character is present in the step name for "Install ESP-IDF via eim" (line 25) and should be removed to prevent YAML parsing errors. Additionally, while using 🧰 Tools🪛 YAMLlint (1.35.1)[error] 25-25: syntax error: found character '\t' that cannot start any token (syntax) |
||||||||||||||||||||||||||||||
| - name: Set up Maven | ||||||||||||||||||||||||||||||
| uses: stCarolas/setup-maven@v5 | ||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||
| maven-version: 3.9.6 | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| - name: Set up JDK 17 | ||||||||||||||||||||||||||||||
| uses: actions/setup-java@v3 | ||||||||||||||||||||||||||||||
| uses: actions/setup-java@v4 | ||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||
| java-version: '17' | ||||||||||||||||||||||||||||||
| distribution: 'temurin' | ||||||||||||||||||||||||||||||
| cache: 'maven' | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| - name: Build with Maven | ||||||||||||||||||||||||||||||
| run: export NO_AT_BRIDGE=1 && mvn clean verify -Djarsigner.skip=true spotbugs:spotbugs -DskipTests=false -DtestWorkspace=/opt/actions-runner/_work/workspace | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package com.espressif.idf.core.tools; | ||
|
|
||
| public interface EimConstants | ||
| { | ||
| String EIM_WIN_PATH = "C:\\Espressif\\tools\\esp_ide.json"; //$NON-NLS-1$ | ||
|
||
|
|
||
| String EIM_URL = "https://dl.espressif.com/dl/eim/"; //$NON-NLS-1$ | ||
|
|
||
| String EIM_POSIX_PATH = System.getProperty("user.home").concat("/.espressif/tools/eim_idf.json"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
|
|
||
| String INSTALL_TOOLS_FLAG = "INSTALL_TOOLS_FLAG"; //$NON-NLS-1$ | ||
|
|
||
| String TOOL_SET_CONFIG_LEGACY_CONFIG_FILE = "tool_set_config.json"; //$NON-NLS-1$ | ||
|
|
||
| String OLD_CONFIG_EXPORTED_FLAG = "OLD_CONFIG_EXPORTED_FLAG"; //$NON-NLS-1$ | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,47 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.espressif.idf.core.tools; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.FileReader; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.eclipse.core.runtime.Platform; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.espressif.idf.core.tools.vo.EimJson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.gson.Gson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.google.gson.GsonBuilder; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class EimIdfConfiguratinParser | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+12
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix class name typo. The class name contains a typo: "Configuratin" should be "Configuration". -public class EimIdfConfiguratinParser
+public class EimIdfConfigurationParser📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private EimJson eimJson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private Gson gson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public EimIdfConfiguratinParser() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| gson = new GsonBuilder().setPrettyPrinting().enableComplexMapKeySerialization() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .excludeFieldsWithoutExposeAnnotation().create(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private void load() throws IOException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try (FileReader fileReader = new FileReader( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Platform.getOS().equals(Platform.OS_WIN32) ? EimConstants.EIM_WIN_PATH : EimConstants.EIM_POSIX_PATH)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eimJson = gson.fromJson(fileReader, EimJson.class); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve error handling in load() method. The load method should handle JSON parsing errors and file not found scenarios more gracefully. private void load() throws IOException
{
+ File configFile = new File(
+ Platform.getOS().equals(Platform.OS_WIN32) ?
+ EimConstants.EIM_WIN_PATH :
+ EimConstants.EIM_POSIX_PATH
+ );
+
+ if (!configFile.exists()) {
+ throw new FileNotFoundException("EIM configuration file not found: " + configFile.getPath());
+ }
+
try (FileReader fileReader = new FileReader(
- Platform.getOS().equals(Platform.OS_WIN32) ? EimConstants.EIM_WIN_PATH : EimConstants.EIM_POSIX_PATH))
+ configFile))
{
eimJson = gson.fromJson(fileReader, EimJson.class);
+ if (eimJson == null) {
+ throw new IOException("Failed to parse EIM configuration file");
+ }
+ } catch (JsonSyntaxException e) {
+ throw new IOException("Invalid JSON format in EIM configuration file", e);
}
}📝 Committable suggestion
Suggested change
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you also handle eim file not found, parsing exception issues and version specific things(not sure if it's already introduced version number the eim_ide.json file) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public EimJson getEimJson(boolean reload) throws IOException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (reload) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| load(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (eimJson == null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| load(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return eimJson; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package com.espressif.idf.core.tools; | ||
|
|
||
| import org.eclipse.osgi.util.NLS; | ||
|
|
||
|
|
||
| public class Messages extends NLS | ||
| { | ||
| private static final String BUNDLE_NAME = "com.espressif.idf.core.tools.messages"; //$NON-NLS-1$ | ||
|
|
||
| public static String InstallToolsHandler_CopyingOpenOCDRules; | ||
| public static String InstallToolsHandler_OpenOCDRulesCopyWarning; | ||
| public static String InstallToolsHandler_OpenOCDRulesCopyWarningMessage; | ||
| public static String InstallToolsHandler_OpenOCDRulesNotCopied; | ||
| public static String InstallToolsHandler_OpenOCDRulesCopyPaths; | ||
| public static String InstallToolsHandler_OpenOCDRulesCopyError; | ||
| public static String InstallToolsHandler_OpenOCDRulesCopied; | ||
|
|
||
| static | ||
| { | ||
| // initialize resource bundle | ||
| NLS.initializeMessages(BUNDLE_NAME, Messages.class); | ||
| } | ||
|
|
||
| private Messages() | ||
| { | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Custom runner labels need documentation
The custom labels
eclipseandBrnoUBU0004are indeed undocumented. They are only used in ci.yml for a self-hosted runner but their purpose and requirements are not explained anywhere in the repository. The original suggestions remain valid:.github/actionlint.yamlto validate workflow syntaxeclipselabel represents (e.g., required Eclipse development environment)BrnoUBU0004represents (specific Ubuntu machine in Brno)🔗 Analysis chain
Document custom runner labels.
The custom labels
eclipseandBrnoUBU0004should be documented:.github/actionlint.yaml🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 352
Script:
Length of output: 68250
Script:
Length of output: 1314
🧰 Tools
🪛 actionlint (1.7.4)
15-15: label "eclipse" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
15-15: label "BrnoUBU0004" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)