#2497 Add display format as Hex numbers or binary numbers #3146
Conversation
- Updated use of @XssProtect annotation; - Corrected definition tests in build.gradle for XssValidatorUtilsTestsSuite;
- Corrected UserProfilesAPI.java
- Corrected EventDetectorDTO.java;
- Added support for List and Map for XssProtect annotations; - Corrected protect in: FolderPointHierarchy, CompoundComponentDTO;
- Added support array for XssProtect annotation; - Corrected protect for JsonEventSearch;
- Fixed select 'Add user' in MailingList; - Added: UserIdentifier, AddressEntryJson, EmailRecipientJson, MailingListJson, UserEntryJson; - Reduced data users in: ViewDwr, WatchListDwr, UsersDwr, ReportsDwr, MailingListsDwr, EventHandlersDwr, DataPointDetailsController;
…de_user_information_in_DWR
- Corrected MailingListsDwr.getMailingList(int id); - ReportsDwr.init;
Release/2.8.0
…4_Updated_REST_API_with_incoming_data_for_XSS_protection
…de_user_information_in_DWR
…on_in_DWR Fix/#3178 hide user information in dwr
…_incoming_data_for_XSS_protection
…I_with_incoming_data_for_XSS_protection Feature/#3174 updated rest api with incoming data for xss protection
- Added block packages: java.lang.Process.* java.lang.invoke..* java.io..* java.nio..* - Added access packages: org.scada_lts..* com.serotonin..* cc.radiuino.scadabr..* br.org.scadabr..* java.util..* java.lang..* org.springframework.jdbc.core..* - Added tests to ScriptExecutorTest;
- Added block packages: java.lang.Process.* java.lang.invoke..* java.io..* java.nio..* - Added access packages: java.util..* - Added tests to ScriptExecutorTest;
- Added block packages: java.lang.Process.* java.lang.invoke..* java.io..* java.nio..* - Added access packages: java.util..* - Added tests to ScriptExecutorTest;
- Refactor ScriptsAPI - using specially created exceptions for the REST API;
- Deprecated ScriptController and endpoint /script/execute/{xid} (use: /api/scripts/execute/{xid})
- Added or corrected name tests ScriptExecutorTestsSuite: JavaClassDeniedScriptExecutorTest,
JavaClassGrantedScriptExecutorTest,
JsScriptExecutorTest;
- Added migration: V2_8_1__SynopticPanelChangeDataTypeForColumnVectorImage
…t time: - Corrected time for return to normal for DifferenceDetectorRT(NoChangeDetectorRT, NoUpdateDetectorRT);
- Corrected generate keystore added applicationUri, corrected other attributes; - Set 20 for search depth limit;
- Fixed leak threads;
- Revert scadalts.security.js.access.granted.class.regexes
- Removed import org.scada_lts.utils.PathSecureUtils in SvgProcessingUtils;
- Moved NettyTerminateUtils to impl package; - Added PollingDataSourceUtils.terminateNetty;
This reverts commit 1c5f9c7.
This reverts commit d8d774f.
- Moved NettyTerminateUtils to impl package; - Added PollingDataSourceUtils.terminateNetty; - Update docker-compose.yml, docker-compose-dev.yml;
- Created annotation java bean validation @scriptprotect: ScriptConstraintListValidator,ScriptValidator, ScriptValidatorUtils; - Set @scriptprotect annotation: MetaPointLocatorJson, JsonScript,MetaPointLocatorVO, ScriptComponent, ScriptVO, PointLinkVO, ViewScriptComponentDTO; - Set @XssProtect annotation: DataPointVO, UserComment, ImplDefinition, PointComponent, ScriptVO, ContextualizedScriptVO, ButtonComponent, ScriptButtonComponent, BinaryEventTextRenderer, MultistateEventValue, RangeEventValue, AlphanumericValue, PointEventDetectorVO, PointLinkVO; - Added validation before execute script: ScriptExecutor, ContextualizedScriptRT; - Added script validation before save: MetaPointLocatorVO, PointLinkVO, ScriptVO; - Added classes: SandboxClassShutter, ScriptContextUtils; - Added exception: ScriptLocalizableException; - Refactor ScriptsAPI.java; - Removed deprecated classes: PointView.java, AnalogImageSetRenderer.java BaseGraphicRenderer.java BasicImageRenderer.java BasicRenderer.java BinaryImageSetRenderer.java DynamicImageRenderer.java GraphicRenderer.java ImageSetRenderer.java MultistateImageSetRenderer.java ScriptRenderer.java ThumbnailRenderer.java;
- Fixed endpoint to test script, added: ScriptsAPI.executeScriptTest - Corrected exception handler for Java Bean Validation error;
- Fixed endpoint /api/scripts/execute-test returned String message.
- Support deprecated endpoint /script; - Added check isSecurityJsValidatorEnabled before validation;
- Added property scadalts.security.js.filter.enabled in env.properties;
…are_OPC_Server #3213 How Connect to Kepware OPC Server:
…ting Fix/#3214 safe mode for scripting
- Corrected HeadersFromSystemSettingsWriter, if a given parameter exists, it is updated;
This reverts commit 6a4d676.
…_in_AuthenticationAPI #3225 Wrong endpoint type in AuthenticationAPI
…ite_attribute_for_session_cookie #3228 Configure SameSite attribute for session cookie
- Corrected V2_8_1__SynopticPanelChangeDataTypeForColumnVectorImage;
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request introduces two new numeric text renderer types—Binary and Hexadecimal—into the text rendering system. The changes register these renderers in the infrastructure, wire them into the DWR-based UI save flow, update the JSP renderer interface, and add internationalization support across multiple language locales. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/com/serotonin/mango/view/text/BaseTextRenderer.java:
- Around line 53-54: The Factory.getType() method in BaseTextRenderer is missing
handling for BinaryRenderer and HexadecimalRenderer, causing a
ShouldNeverHappenException for those types; update Factory.getType() (the
if/else chain) to check for BinaryRenderer.class and HexadecimalRenderer.class
and return the corresponding type strings that match
BinaryRenderer.getDefinition() and HexadecimalRenderer.getDefinition(), placing
these checks before the final else/exception branch so JSON import/export
recognizes these renderer types.
In @webapp-resources/messages_pl.properties:
- Around line 3596-3598: The Polish locale file contains English values for the
new keys textRenderer.binary.number and textRenderer.hex; update
messages_pl.properties to provide proper Polish translations for these keys
(e.g., "Liczba binarna" for textRenderer.binary.number and "Szesnastkowy" or
"Szestnastkowy" for textRenderer.hex) or confirm that English is acceptable and
leave as-is; ensure you edit the entries for textRenderer.binary.number and
textRenderer.hex (and adjust systemSettings.scadaConfCheck if needed) so the PL
file consistently contains Polish strings.
🧹 Nitpick comments (2)
webapp-resources/messages_fr.properties (1)
3453-3455: English translations in French properties file.The new translation keys are in English rather than French. While this is acceptable per project conventions (French is not actively maintained), consider adding proper French translations for consistency with existing keys:
systemSettings.scadaConfCheck=Vérifier la configurationtextRenderer.binary.number=Nombre binairetextRenderer.hex=HexadécimalBased on learnings, non-maintained locales like French can contain English text and this should not block the PR.
src/com/serotonin/mango/web/dwr/DataPointEditDwr.java (1)
112-115: Fix formatting to match existing code style.The new methods are functionally correct but use inconsistent formatting compared to the rest of the file. Other setter methods in this file use multi-line formatting with proper indentation.
♻️ Suggested formatting fix
- public void setHexadecimalRenderer() {setTextRenderer(new HexadecimalRenderer()); } - - public void setBinaryRenderer(){setTextRenderer(new BinaryRenderer());} - + public void setHexadecimalRenderer() { + setTextRenderer(new HexadecimalRenderer()); + } + + public void setBinaryRenderer() { + setTextRenderer(new BinaryRenderer()); + } +
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
WebContent/WEB-INF/jsp/pointEdit/textRenderer.jspsrc/com/serotonin/mango/view/text/BaseTextRenderer.javasrc/com/serotonin/mango/view/text/BinaryRenderer.javasrc/com/serotonin/mango/view/text/BinaryTextRenderer.javasrc/com/serotonin/mango/view/text/HexadecimalRenderer.javasrc/com/serotonin/mango/web/dwr/DataPointEditDwr.javawebapp-resources/messages_de.propertieswebapp-resources/messages_en.propertieswebapp-resources/messages_es.propertieswebapp-resources/messages_fi.propertieswebapp-resources/messages_fr.propertieswebapp-resources/messages_lu.propertieswebapp-resources/messages_nl.propertieswebapp-resources/messages_pl.propertieswebapp-resources/messages_pt.propertieswebapp-resources/messages_ru.propertieswebapp-resources/messages_zh.properties
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-05T14:39:51.606Z
Learnt from: Limraj
Repo: SCADA-LTS/Scada-LTS PR: 3239
File: webapp-resources/messages_fi.properties:3565-3565
Timestamp: 2026-01-05T14:39:51.606Z
Learning: SCADA-LTS maintains translations only for Polish and English. Do not invest in or expect active maintenance for other languages (e.g., messages_fi.properties, messages_de.properties). If a messages_*.properties file contains English text and is not maintained, it can be left as-is for now. AI translation may be used in the future. During reviews, prioritize Polish and English keys/text; non-maintained locales should not block changes.
Applied to files:
webapp-resources/messages_zh.propertieswebapp-resources/messages_en.propertieswebapp-resources/messages_es.propertieswebapp-resources/messages_fi.propertieswebapp-resources/messages_de.propertieswebapp-resources/messages_nl.propertieswebapp-resources/messages_pt.propertieswebapp-resources/messages_pl.propertieswebapp-resources/messages_ru.propertieswebapp-resources/messages_fr.propertieswebapp-resources/messages_lu.properties
📚 Learning: 2026-01-12T08:44:55.232Z
Learnt from: Limraj
Repo: SCADA-LTS/Scada-LTS PR: 3239
File: src/br/org/scadabr/view/component/ChartComparatorComponent.java:63-66
Timestamp: 2026-01-12T08:44:55.232Z
Learning: In ChartComparatorComponent, data point select dropdowns are intentionally rendered with Collections.emptyList() and populated client-side using lazy loading via the DataPointsSelect JavaScript class, initialized by loadPointsSelects("select[id^=chartComparator]", ...) in viewEdit.jsp and views.jsp.
Applied to files:
src/com/serotonin/mango/web/dwr/DataPointEditDwr.java
🧬 Code graph analysis (2)
src/com/serotonin/mango/view/text/HexadecimalRenderer.java (1)
src/com/serotonin/mango/view/text/BinaryRenderer.java (1)
JsonRemoteEntity(12-79)
src/com/serotonin/mango/view/text/BinaryRenderer.java (2)
src/com/serotonin/mango/view/text/BinaryTextRenderer.java (1)
JsonRemoteEntity(40-168)src/com/serotonin/mango/view/text/HexadecimalRenderer.java (1)
JsonRemoteEntity(13-72)
🔇 Additional comments (18)
webapp-resources/messages_pt.properties (1)
3612-3613: English text in Portuguese locale file - acceptable per project guidelines.The new i18n keys use English values ("Binary number", "Hexadecimal") rather than Portuguese translations. While most other entries in this file are properly localized to Portuguese, this inconsistency is acceptable.
Based on learnings, SCADA-LTS actively maintains only Polish and English translations. Non-maintained locales like Portuguese may contain English text and this should not block the changes. AI translation may be used in the future to localize these entries.
For future reference, Portuguese translations would be:
textRenderer.binary.number=Número bináriotextRenderer.hex=Hexadecimal(same in Portuguese)webapp-resources/messages_lu.properties (1)
3473-3474: LGTM! English text in non-maintained locale is acceptable.The new localization keys for Binary and Hexadecimal text renderers have been added correctly. While the values are in English rather than Luxembourgish, this is acceptable for this locale file.
Based on learnings, SCADA-LTS maintains translations only for Polish and English. Non-maintained locales like
messages_lu.propertiescan contain English text and should not block feature changes. AI translation may be applied to these files in the future.The keys follow the existing naming convention (
textRenderer.*) and align with the newly introduced Binary and Hexadecimal text renderers mentioned in the PR context.webapp-resources/messages_zh.properties (1)
3559-3561: LGTM!The new localization keys
textRenderer.binary.numberandtextRenderer.hexare correctly added to support the new Binary and Hexadecimal text renderers. Based on learnings, English text for non-maintained locales (like Chinese) is acceptable as SCADA-LTS only actively maintains Polish and English translations.src/com/serotonin/mango/view/text/BinaryTextRenderer.java (1)
42-43: LGTM!The display name change from
"BINARY"to"BINARY (T/F)"appropriately disambiguates this existing true/false label renderer from the newBinaryRendererthat displays numeric values as binary numbers. TheTYPE_NAMEconstant remains unchanged, preserving backward compatibility for serialization and configuration.webapp-resources/messages_de.properties (1)
3456-3458: I18n keys are properly integrated with their runtime lookups.All three keys are correctly referenced in the Java source:
textRenderer.hex→HexadecimalRenderer.javatextRenderer.binary.number→BinaryRenderer.javasystemSettings.scadaConfCheck→ All property filesNo discrepancies found. The entries are consistent across all 10 locale files.
webapp-resources/messages_nl.properties (1)
3574-3576: Verify inclusion ofsystemSettings.scadaConfCheckentry and confirm English text is intentional.Three new translation entries have been added to the Dutch locale file:
Lines 3575-3576 (
textRenderer.binary.numberandtextRenderer.hex) align with the PR's stated objective of adding binary and hexadecimal display format support.Line 3574 (
systemSettings.scadaConfCheck=Check configuration) appears unrelated to the binary/hex renderer feature described in the PR title. Please verify whether this entry:
- Is intentionally included as part of this PR
- Should be removed as an accidental inclusion from another change
- Is actually related to the feature but not mentioned in the PR description
All three entries use English text in a Dutch translation file (
messages_nl.properties). Based on learnings, SCADA-LTS only actively maintains Polish and English translations, so English placeholders in other locale files are acceptable and should not block this PR.Based on learnings, Dutch translations are not actively maintained in SCADA-LTS, so English text in this file is acceptable.
webapp-resources/messages_ru.properties (1)
3607-3609: Key names verified and correct.All three keys are properly referenced in the codebase:
textRenderer.binary.number→ referenced inBinaryRenderer.javatextRenderer.hex→ referenced inHexadecimalRenderer.javasystemSettings.scadaConfCheck→ used across the applicationThe English values are consistent across all 11 locale files, so this is not a Russian-specific issue. If desired, these entries could be translated to Russian for completeness, but the English fallback will function without runtime warnings.
webapp-resources/messages_en.properties (2)
3461-3461: LGTM! Clean addition for hexadecimal renderer.The key follows the established naming convention and the translation value is clear and appropriate.
3460-3460: No changes needed. The keystextRenderer.binaryandtextRenderer.binary.numberare intentionally distinct and serve different renderers:BinaryTextRenderer(for boolean binary values) andBinaryRenderer(for numeric binary representation), respectively. This hierarchical naming is consistent across all 11 language files and is a deliberate design choice.webapp-resources/messages_es.properties (1)
3500-3501: LGTM!The new translation keys for binary and hexadecimal renderers are correctly added. Based on learnings, Spanish is not an actively maintained locale (only Polish and English are), so English text is acceptable here.
src/com/serotonin/mango/web/dwr/DataPointEditDwr.java (1)
33-33: Wildcard import is acceptable here.Given that this file uses multiple classes from the
com.serotonin.mango.view.textpackage (AnalogRenderer, BinaryTextRenderer, MultistateRenderer, NoneRenderer, PlainRenderer, RangeRenderer, TimeRenderer, and now HexadecimalRenderer and BinaryRenderer), the wildcard import is reasonable.webapp-resources/messages_fi.properties (1)
3583-3586: New translation keys added correctly.The new keys
textRenderer.binary.numberandtextRenderer.hexare properly added for the new renderers. Based on learnings, Finnish translations are not actively maintained, so using English text here is acceptable.WebContent/WEB-INF/jsp/pointEdit/textRenderer.jsp (2)
163-166: Empty containers added correctly for configuration-less renderers.The empty
<tbody>elements are appropriate since these renderers have no user-configurable options.
255-258: Save logic wired correctly to DWR methods.The save flow properly calls
DataPointEditDwr.setHexadecimalRenderer(callback)andDataPointEditDwr.setBinaryRenderer(callback)for the new renderer types.src/com/serotonin/mango/view/text/HexadecimalRenderer.java (2)
47-55: Implementation looks correct for typical numeric values.The conversion logic is straightforward. Note that:
- Fractional values are truncated (e.g.,
3.7→0x3)- Negative numbers are rendered as two's complement (e.g.,
-1→0xFFFFFFFFFFFFFFFF)This behavior seems intentional for low-level/bit-level display use cases, but consider documenting this in a comment if it's the expected behavior.
1-72: New HexadecimalRenderer follows established patterns.The class structure, serialization, and integration with the text renderer framework are consistent with existing renderers.
src/com/serotonin/mango/view/text/BinaryRenderer.java (2)
46-62: Binary formatting with 4-bit grouping is well-implemented.The algorithm correctly pads to the nearest 4-bit boundary and groups bits with spaces for readability (e.g.,
255→"1111 1111",5→"0101").Same edge case behavior as HexadecimalRenderer applies: fractional truncation and two's complement for negative values.
1-79: New BinaryRenderer follows established patterns and is consistent with HexadecimalRenderer.The class structure, serialization, and framework integration are correct.
| d.add(BinaryRenderer.getDefinition()); | ||
| d.add(HexadecimalRenderer.getDefinition()); |
There was a problem hiding this comment.
New renderer definitions registered correctly, but Factory is incomplete.
The new renderers are properly added to the definitions list. However, the Factory.getType() method (lines 203-218) lacks handling for BinaryRenderer and HexadecimalRenderer. When importing/exporting JSON with these renderer types, the code will throw ShouldNeverHappenException("What's this?: ...") because the if-else chain doesn't include them.
🐛 Proposed fix to add missing cases in Factory.getType()
Add the following cases before the else clause at line 217:
else if (def == TimeRenderer.getDefinition())
clazz = TimeRenderer.class;
+ else if (def == BinaryRenderer.getDefinition())
+ clazz = BinaryRenderer.class;
+ else if (def == HexadecimalRenderer.getDefinition())
+ clazz = HexadecimalRenderer.class;
else
throw new ShouldNeverHappenException("What's this?: " + def.getName());🤖 Prompt for AI Agents
In @src/com/serotonin/mango/view/text/BaseTextRenderer.java around lines 53 -
54, The Factory.getType() method in BaseTextRenderer is missing handling for
BinaryRenderer and HexadecimalRenderer, causing a ShouldNeverHappenException for
those types; update Factory.getType() (the if/else chain) to check for
BinaryRenderer.class and HexadecimalRenderer.class and return the corresponding
type strings that match BinaryRenderer.getDefinition() and
HexadecimalRenderer.getDefinition(), placing these checks before the final
else/exception branch so JSON import/export recognizes these renderer types.
| systemSettings.scadaConfCheck=Check configuration | ||
| textRenderer.binary.number=Binary number | ||
| textRenderer.hex=Hexadecimal |
There was a problem hiding this comment.
Polish locale: consider providing actual PL translations for the new renderer labels.
textRenderer.binary.number and textRenderer.hex are added, but values are still English. If messages_pl.properties is expected to be maintained, please translate these strings (or confirm English is acceptable here going forward). Based on learnings, prioritize Polish/English correctness.
🤖 Prompt for AI Agents
In @webapp-resources/messages_pl.properties around lines 3596 - 3598, The Polish
locale file contains English values for the new keys textRenderer.binary.number
and textRenderer.hex; update messages_pl.properties to provide proper Polish
translations for these keys (e.g., "Liczba binarna" for
textRenderer.binary.number and "Szesnastkowy" or "Szestnastkowy" for
textRenderer.hex) or confirm that English is acceptable and leave as-is; ensure
you edit the entries for textRenderer.binary.number and textRenderer.hex (and
adjust systemSettings.scadaConfCheck if needed) so the PL file consistently
contains Polish strings.
…_Hex_numbers_or_binary_numbers
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.