-
Notifications
You must be signed in to change notification settings - Fork 121
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
IEP-974: Adding the IDE env versions to the esp_idf.json #781
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. Suggestions needed on how we can improve this message |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,5 +17,5 @@ CMakeErrorParser_NotAWorkspaceResource=Could not map %s to a workspace resource. | |
IDFBuildConfiguration_CMakeBuildConfiguration_NoToolchainFile=No toolchain file found | ||
IncreasePartitionSizeTitle=Low Application Partition Size | ||
IncreasePartitionSizeMessage=Less than 30% of application partition size is free({0} of {1} bytes), would you like to increase it? Please click <a href={2}>here</a> to check more details. | ||
ToolsInitializationDifferentPathMessageBoxTitle=Different IDF path found in the config file | ||
ToolsInitializationDifferentPathMessageBoxMessage=A different IDF path was found in the startup configuration file do you want to install the tools in this path or keep the older path\nNew Path: {0}\nOld Path: {1} | ||
ToolsInitializationDifferentPathMessageBoxTitle=Different IDF path found in IDE Environment | ||
ToolsInitializationDifferentPathMessageBoxMessage=Different IDF tools are installed in the IDE Environment: {0} than the one selected in startup config file for IDE (esp_idf.json).\nIf you want to keep the IDF tools in the IDE Environment select Yes(This will also add the tools to esp_idf.json).\nIf you want to revert to the last used tools in esp_idf.json press No. | ||
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. what about rephrasing it like this:
However, I found version 1 too verbose
what do you think? 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. The second version is misleading to my understanding as I think it can confuse the user like they can press No thinking that the tools wont be added to json file but instead we are trying to revert the tools back to the original installation. This would be okay if we didnt have the usecase where the tools are updated via the IDE offline installer. |
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. Needed to use gson from google's api to make the file output pretty. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,11 +36,16 @@ | |
import com.espressif.idf.core.logging.Logger; | ||
import com.espressif.idf.core.resources.OpenDialogListenerSupport; | ||
import com.espressif.idf.core.resources.ResourceChangeListener; | ||
import com.espressif.idf.core.util.IDFUtil; | ||
import com.espressif.idf.core.util.StringUtil; | ||
import com.espressif.idf.core.util.ToolChainUtil; | ||
import com.espressif.idf.ui.dialogs.MessageLinkDialog; | ||
import com.espressif.idf.ui.update.ExportIDFTools; | ||
import com.espressif.idf.ui.update.InstallToolsHandler; | ||
import com.google.gson.Gson; | ||
import com.google.gson.GsonBuilder; | ||
import com.google.gson.JsonElement; | ||
import com.google.gson.JsonParser; | ||
|
||
@SuppressWarnings("restriction") | ||
public class InitializeToolsStartup implements IStartup | ||
|
@@ -57,11 +62,10 @@ public class InitializeToolsStartup implements IStartup | |
private static final String IDF_INSTALLED_LIST_KEY = "idfInstalled"; //$NON-NLS-1$ | ||
private static final String PYTHON_PATH = "python"; //$NON-NLS-1$ | ||
private static final String IDF_PATH = "path"; //$NON-NLS-1$ | ||
private static final String IDF_VERSION = "version"; //$NON-NLS-1$ | ||
private static final String IS_INSTALLER_CONFIG_SET = "isInstallerConfigSet"; //$NON-NLS-1$ | ||
private static final String DOC_URL = "\"https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-guides/partition-tables.html?highlight=partitions%20csv#creating-custom-tables\""; //$NON-NLS-1$ | ||
|
||
private String newIdfPath; | ||
|
||
@Override | ||
public void earlyStartup() | ||
{ | ||
|
@@ -102,25 +106,32 @@ public void run() | |
} | ||
else if (isInstallerConfigSet()) | ||
{ | ||
checkForUpdatedVersion(idf_json_file); | ||
if (isInstallerConfigSet()) | ||
if (!isVersionDifferentInFile(idf_json_file)) | ||
{ | ||
Logger.log("Ignoring esp_idf.json settings as it was configured earilier and idf_path is similar."); | ||
return; | ||
} | ||
|
||
Logger.log("A different version for idf tool in ide env and esp_idf.json found"); | ||
IDFEnvironmentVariables idfEnvMgr = new IDFEnvironmentVariables(); | ||
Display.getDefault().syncExec(() -> { | ||
Shell shell = new Shell(Display.getDefault()); | ||
MessageBox messageBox = new MessageBox(shell, SWT.ICON_WARNING | SWT.YES | SWT.NO); | ||
messageBox.setText(Messages.ToolsInitializationDifferentPathMessageBoxTitle); | ||
messageBox.setMessage(MessageFormat.format(Messages.ToolsInitializationDifferentPathMessageBoxMessage, | ||
newIdfPath, idfEnvMgr.getEnvValue(IDFEnvironmentVariables.IDF_PATH))); | ||
idfEnvMgr.getEnvValue(IDFEnvironmentVariables.IDF_PATH))); | ||
int response = messageBox.open(); | ||
if (response == SWT.NO) | ||
if (response == SWT.YES) | ||
{ | ||
IDFEnvironmentVariables idfEnvironmentVariables = new IDFEnvironmentVariables(); | ||
updateEspIdfJsonFile(idf_json_file, idfEnvironmentVariables.getEnvValue(IDFEnvironmentVariables.IDF_PATH)); | ||
String version = IDFUtil.getEspIdfVersion(); | ||
java.util.regex.Pattern p = java.util.regex.Pattern.compile("([0-9][.][0-9])"); //$NON-NLS-1$ | ||
java.util.regex.Matcher m = p.matcher(version); | ||
Comment on lines
+126
to
+127
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. Why not just use Pattern and Matcher and import java.util.regex.Matcher; import java.util.regex.Pattern; here? |
||
if (m.find()) | ||
{ | ||
version = m.group(0); | ||
} | ||
updateEspIdfJsonFile(idf_json_file, idfEnvMgr.getEnvValue(IDFEnvironmentVariables.IDF_PATH), | ||
IDFUtil.getIDFPythonEnvPath(), | ||
version); | ||
Comment on lines
+125
to
+134
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. What if the user updated esp-idf(let's say 5.0 to 5.1) through windows installer, in this case Don't we export the paths in this case? |
||
Preferences prefs = getPreferences(); | ||
prefs.putBoolean(IS_INSTALLER_CONFIG_SET, true); | ||
try | ||
|
@@ -131,8 +142,6 @@ else if (isInstallerConfigSet()) | |
{ | ||
Logger.log(e); | ||
} | ||
|
||
return; | ||
} | ||
}); | ||
} | ||
|
@@ -199,49 +208,72 @@ else if (isInstallerConfigSet()) | |
} | ||
|
||
@SuppressWarnings("unchecked") | ||
private void updateEspIdfJsonFile(File idf_json_file, String newIdfPathToUpdate) | ||
private void updateEspIdfJsonFile(File idf_json_file, String idfPath, String idfPythonPath, String idfVersion) | ||
{ | ||
JSONParser parser = new JSONParser(); | ||
JSONObject jsonObj = null; | ||
try (FileReader reader = new FileReader(idf_json_file)) | ||
{ | ||
jsonObj = (JSONObject) parser.parse(reader); | ||
String idfVersionId = (String) jsonObj.get(IDF_VERSIONS_ID); | ||
JSONObject list = (JSONObject) jsonObj.get(IDF_INSTALLED_LIST_KEY); | ||
if (list == null) | ||
{ | ||
return; | ||
} | ||
// selected esp-idf version information | ||
JSONObject selectedIDFInfo = (JSONObject) list.get(idfVersionId); | ||
selectedIDFInfo.put(IDF_PATH, newIdfPathToUpdate); | ||
list.put(idfVersionId, selectedIDFInfo); | ||
|
||
// verifying if the paths are already in the file we just update the idfSelectedId to that version in esp_idf.json | ||
for (Object key : list.keySet()) | ||
{ | ||
JSONObject idfInstalled = (JSONObject) list.get(key); | ||
if (idfInstalled.get(IDF_PATH).equals(idfPath) && idfInstalled.get(PYTHON_PATH).equals(idfPythonPath) | ||
&& idfInstalled.get(IDF_VERSION).equals(idfVersion)) | ||
{ | ||
jsonObj.put(IDF_VERSIONS_ID, key); | ||
// write everything to the file here and return | ||
Logger.log("Already existing paths and versions found in the file with key: " + key); | ||
addJsonToFile(jsonObj, idf_json_file); | ||
return; | ||
} | ||
} | ||
|
||
// since no version was found we create an object into idfInstalled and update the idfSelectedId | ||
JSONObject idfVersionToAdd = new JSONObject(); | ||
idfVersionToAdd.put(IDF_PATH, idfPath); | ||
idfVersionToAdd.put(PYTHON_PATH, idfPythonPath); | ||
idfVersionToAdd.put(IDF_VERSION, idfVersion); | ||
String idfVersionIdToAdd = "esp-idf-" + System.currentTimeMillis(); | ||
list.put(idfVersionIdToAdd, idfVersionToAdd); | ||
jsonObj.put(IDF_INSTALLED_LIST_KEY, list); | ||
jsonObj.put(IDF_VERSIONS_ID, idfVersionIdToAdd); | ||
|
||
Logger.log("Created new object for json with key: " + idfVersionIdToAdd + " This will be added to esp_idf.json"); | ||
addJsonToFile(jsonObj, idf_json_file); | ||
} | ||
catch ( | ||
IOException | ||
| ParseException e) | ||
{ | ||
Logger.log(e); | ||
} | ||
|
||
if (jsonObj != null) | ||
} | ||
|
||
private void addJsonToFile(JSONObject jsonObject, File idf_json_file) throws IOException | ||
{ | ||
if (jsonObject != null) | ||
{ | ||
// using gson for pretty printing | ||
Gson gson = new GsonBuilder().setPrettyPrinting().create(); | ||
JsonElement jsonElement = JsonParser.parseString(jsonObject.toJSONString()); | ||
try (FileWriter fileWriter = new FileWriter(idf_json_file)) | ||
{ | ||
fileWriter.write(jsonObj.toJSONString()); | ||
fileWriter.write(gson.toJson(jsonElement)); | ||
fileWriter.flush(); | ||
|
||
} | ||
catch (IOException e) | ||
{ | ||
Logger.log(e); | ||
} | ||
} | ||
|
||
} | ||
|
||
private void checkForUpdatedVersion(File idf_json_file) | ||
private boolean isVersionDifferentInFile(File idf_json_file) | ||
{ | ||
// read esp-idf.json file | ||
JSONParser parser = new JSONParser(); | ||
|
@@ -250,26 +282,13 @@ private void checkForUpdatedVersion(File idf_json_file) | |
JSONObject jsonObj = (JSONObject) parser.parse(reader); | ||
String idfVersionId = (String) jsonObj.get(IDF_VERSIONS_ID); | ||
JSONObject list = (JSONObject) jsonObj.get(IDF_INSTALLED_LIST_KEY); | ||
if (list == null) | ||
{ | ||
return; | ||
} | ||
// selected esp-idf version information | ||
JSONObject selectedIDFInfo = (JSONObject) list.get(idfVersionId); | ||
String idfPath = (String) selectedIDFInfo.get(IDF_PATH); | ||
IDFEnvironmentVariables idfEnvMgr = new IDFEnvironmentVariables(); | ||
if (idfEnvMgr.getEnvValue(IDFEnvironmentVariables.IDF_PATH).equals(idfPath)) | ||
return; | ||
newIdfPath = idfPath; | ||
Preferences prefs = getPreferences(); | ||
prefs.putBoolean(IS_INSTALLER_CONFIG_SET, false); | ||
try | ||
{ | ||
prefs.flush(); | ||
} | ||
catch (BackingStoreException e) | ||
if (list != null) | ||
{ | ||
Logger.log(e); | ||
// selected esp-idf version information | ||
JSONObject selectedIDFInfo = (JSONObject) list.get(idfVersionId); | ||
String idfPath = (String) selectedIDFInfo.get(IDF_PATH); | ||
IDFEnvironmentVariables idfEnvMgr = new IDFEnvironmentVariables(); | ||
return !idfEnvMgr.getEnvValue(IDFEnvironmentVariables.IDF_PATH).equals(idfPath); | ||
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. idfEnvMgr.getEnvValue(IDFEnvironmentVariables.IDF_PATH) will return null when IDE is not configured with the IDF_PATH(IDE is not launched and configured not even once). That leads to NPE 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. In case of multiple back slashes in the path, this might fail. 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. good catch. Probably we could use Paths.get().normalize() here 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.
I dont think using normalize is a good idea simply comparison like using |
||
} | ||
} | ||
catch ( | ||
|
@@ -279,6 +298,7 @@ private void checkForUpdatedVersion(File idf_json_file) | |
Logger.log(e); | ||
} | ||
|
||
return false; | ||
} | ||
|
||
private Preferences getPreferences() | ||
|
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.
Please see if this is fine here or we can somehow add it to main README.MD