From 425c1db13281d9fb6ac5aa1e3120cc60565f1d95 Mon Sep 17 00:00:00 2001 From: jpirnay Date: Mon, 20 Apr 2026 21:16:31 +0200 Subject: [PATCH] Review comments --- lib/I18n/translations/english.yaml | 3 ++ .../settings/ClockSettingsActivity.cpp | 30 +------------------ .../settings/DetectTimezoneActivity.cpp | 4 +++ src/activities/settings/SettingInfo.h | 6 ++++ src/network/CrossPointWebServer.cpp | 3 +- 5 files changed, 16 insertions(+), 30 deletions(-) diff --git a/lib/I18n/translations/english.yaml b/lib/I18n/translations/english.yaml index 492c76ac..b7836f90 100644 --- a/lib/I18n/translations/english.yaml +++ b/lib/I18n/translations/english.yaml @@ -126,6 +126,9 @@ STR_TZ_EST: "US Eastern (EST/EDT)" STR_TZ_CST: "US Central (CST/CDT)" STR_TZ_MST: "US Mountain (MST/MDT)" STR_TZ_PST: "US Pacific (PST/PDT)" +STR_TZ_AST_ADT: "Atlantic Canada (AST/ADT)" +STR_TZ_ACST_ACDT: "Australia Central (ACST/ACDT)" +STR_TZ_AKST_AKDT: "Alaska (AKST/AKDT)" STR_TZ_AEST: "Australia Eastern (AEST/AEDT)" STR_TZ_NZST: "New Zealand (NZST/NZDT)" STR_TZ_MSK: "Russia (MSK)" diff --git a/src/activities/settings/ClockSettingsActivity.cpp b/src/activities/settings/ClockSettingsActivity.cpp index 8ec04d42..f0d64a1f 100644 --- a/src/activities/settings/ClockSettingsActivity.cpp +++ b/src/activities/settings/ClockSettingsActivity.cpp @@ -13,15 +13,6 @@ #include "components/UITheme.h" #include "fontIds.h" -namespace { -const StrId timeZoneNames[CrossPointSettings::TIMEZONE_COUNT] = { - StrId::STR_TZ_UTC, StrId::STR_TZ_CET, StrId::STR_TZ_EET, StrId::STR_TZ_MSK, - StrId::STR_TZ_UTC_PLUS4, StrId::STR_TZ_IST, StrId::STR_TZ_UTC_PLUS7, StrId::STR_TZ_UTC_PLUS8, - StrId::STR_TZ_UTC_PLUS9, StrId::STR_TZ_AEST, StrId::STR_TZ_NZST, StrId::STR_TZ_UTC_MINUS3, - StrId::STR_TZ_EST, StrId::STR_TZ_CST, StrId::STR_TZ_MST, StrId::STR_TZ_PST, - StrId::STR_TZ_UTC, StrId::STR_TZ_UTC, StrId::STR_TZ_UTC}; -} // namespace - void ClockSettingsActivity::buildMenuItems() { menuItems.reserve(6); menuItems.push_back(SettingInfo::Separator(StrId::STR_SETTINGS_TITLE)); @@ -35,27 +26,8 @@ void ClockSettingsActivity::buildMenuItems() { {StrId::STR_TZ_UTC, StrId::STR_TZ_CET, StrId::STR_TZ_EET, StrId::STR_TZ_MSK, StrId::STR_TZ_UTC_PLUS4, StrId::STR_TZ_IST, StrId::STR_TZ_UTC_PLUS7, StrId::STR_TZ_UTC_PLUS8, StrId::STR_TZ_UTC_PLUS9, StrId::STR_TZ_AEST, StrId::STR_TZ_NZST, StrId::STR_TZ_UTC_MINUS3, StrId::STR_TZ_EST, StrId::STR_TZ_CST, - StrId::STR_TZ_MST, StrId::STR_TZ_PST, StrId::STR_TZ_UTC, StrId::STR_TZ_UTC, StrId::STR_TZ_UTC}, + StrId::STR_TZ_MST, StrId::STR_TZ_PST, StrId::STR_TZ_AST_ADT, StrId::STR_TZ_ACST_ACDT, StrId::STR_TZ_AKST_AKDT}, "timeZone", StrId::STR_CAT_SYSTEM); - tzSetting.enumLabels = {"UTC (GMT/BST)", - "Central European (CET/CEST)", - "Eastern European (EET/EEST)", - "Moscow (MSK)", - "UTC+4", - "India (IST)", - "UTC+7", - "UTC+8", - "UTC+9", - "Australia Eastern (AEST/AEDT)", - "New Zealand (NZST/NZDT)", - "UTC-3", - "Eastern US/Canada (EST/EDT)", - "Central US/Canada (CST/CDT)", - "Mountain US/Canada (MST/MDT)", - "Pacific US/Canada (PST/PDT)", - "Atlantic Canada (AST/ADT)", - "Australia Central (ACST/ACDT)", - "Alaska (AKST/AKDT)"}; menuItems.push_back(std::move(tzSetting)); } diff --git a/src/activities/settings/DetectTimezoneActivity.cpp b/src/activities/settings/DetectTimezoneActivity.cpp index ed2110cf..2f1560ee 100644 --- a/src/activities/settings/DetectTimezoneActivity.cpp +++ b/src/activities/settings/DetectTimezoneActivity.cpp @@ -65,6 +65,10 @@ bool mapIanaTimezone(const std::string& tz, uint8_t& outSetting) { outSetting = TZ::TZ_AKST_AKDT; return true; } + if (tz == "Australia/Adelaide" || tz == "Australia/Broken_Hill") { + outSetting = TZ::TZ_ACST_ACDT; + return true; + } if (tz == "America/Sao_Paulo" || tz == "America/Argentina/Buenos_Aires" || tz == "America/Montevideo") { outSetting = TZ::TZ_UTC_MINUS3; return true; diff --git a/src/activities/settings/SettingInfo.h b/src/activities/settings/SettingInfo.h index 9baaecdc..d0e684c0 100644 --- a/src/activities/settings/SettingInfo.h +++ b/src/activities/settings/SettingInfo.h @@ -31,6 +31,12 @@ struct SettingInfo { StrId nameId; SettingType type; uint8_t CrossPointSettings::* valuePtr = nullptr; + // Enum values are used as StrId references for localization via I18N.get(). + // If enumLabels is populated, it is authoritative for display and index bounds. + // In that case it must have the same length as enumValues, because consumers + // such as getDisplayValue(), toggleValue(), and CrossPointWebServer::handleGetSettings() + // prefer enumLabels when present. Code paths which validate posted enum values + // should also use enumLabels.size() when enumLabels is non-empty. std::vector enumValues; std::vector enumLabels; SettingAction action = SettingAction::None; diff --git a/src/network/CrossPointWebServer.cpp b/src/network/CrossPointWebServer.cpp index ad04dc13..80261482 100644 --- a/src/network/CrossPointWebServer.cpp +++ b/src/network/CrossPointWebServer.cpp @@ -1325,7 +1325,8 @@ void CrossPointWebServer::handlePostSettings() { } case SettingType::ENUM: { const int val = doc[s.key].as(); - if (val >= 0 && val < static_cast(s.enumValues.size())) { + const auto count = static_cast(s.enumLabels.empty() ? s.enumValues.size() : s.enumLabels.size()); + if (val >= 0 && val < count) { if (s.valuePtr) { SETTINGS.*(s.valuePtr) = static_cast(val); } else if (s.valueSetter) {