-
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-1037: Fix for the confserver handling of the hex values #859
Conversation
WalkthroughThe Java codebase for the SDK Configuration Editor has been updated to improve the handling of hexadecimal values. The changes include displaying long values as hexadecimal strings, modifying UI labels to indicate hexadecimal input, and parsing user input as hexadecimal where appropriate. This enhances the user experience for configurations that require hexadecimal notation. Changes
Poem
TipsChat with CodeRabbit Bot (
|
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.
Self Review
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java (2 hunks)
Additional comments: 1
bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java (1)
- 635-642: The code correctly formats the hexadecimal value with the "0x" prefix and converts it to uppercase. However, it assumes that
newConfigValue
andconfigValue
are of typeLong
. If they are not, aClassCastException
could occur. Ensure that the values are indeedLong
before casting, or handle the potential exception.
....espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java
Show resolved
Hide resolved
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.
LGTM
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java (3 hunks)
Additional comments: 1
bundles/com.espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java (1)
- 853-860: The previous comment about wrapping the
Long.parseLong
call in a try-catch block to handleNumberFormatException
is still valid. The code should be updated to handle this exception gracefully.- modifiedJsonMap.put(configKey, isHex ? Long.parseLong(text.substring(2), 16) : textControl.getText().trim()); + try { + modifiedJsonMap.put(configKey, isHex ? Long.parseLong(text.substring(2), 16) : textControl.getText().trim()); + } catch (NumberFormatException ex) { + // Handle invalid hex number input + // For example, show an error message to the user or log the error + }
....espressif.idf.sdk.config.ui/src/com/espressif/idf/sdk/config/ui/SDKConfigurationEditor.java
Show resolved
Hide resolved
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.
LGTM
@alirana01 hi ! Tested under: After build I do I see a match in the sdk files |
Description
Fixed the conf server so hex values are shown properly and are handled gracefully as hex values
Original reported issue that can be used for testing is here
Fixes # (IEP-1037)
Type of change
Please delete options that are not relevant.
How has this been tested?
Open the sdkconfig and try to see the hex value properties like Partition Table -> Offset of partition table. It should be shown in Hex format and any change must be saved and reflected once you close and open the sdkconfig editor.
Please see that other hex properties are also safely saved as well. You can verify the changed value in menuconfig via terminal to see that correct hex is stored
Test Configuration:
Checklist
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes