From b25389b43e23e2200aad196cfd67fa72f8bbc599 Mon Sep 17 00:00:00 2001 From: Zach Nelson Date: Fri, 1 May 2026 09:39:28 -0500 Subject: [PATCH] refactor: Move language setting into JSON settings (#1796) ## Summary Another follow up to #1408. That PR revised the language.bin settings file to version 2. After merging, it occurred to me that it would make more sense to have the language inside the overall settings.json file, easy for users to see and change directly. This change adds migration from language.bin to settings.json. It only migrates from the pre-#1408 v1 language.bin format, because v2 has only been in for a couple of commits and I don't think we need long-term maintenance code to handle it. #1408 had a potential long-term maintenance issue for migration. It assumed that the enum order of languages never changes, only grows. So, e.g. "Portuguese (Portugal)" could never be added next to "Portuguese (Brazil)", only appended. This change includes a hardcoded frozen ordering of the language indices as they existed at 2f969a9, and migrates to storing a language code string in the settings.json instead for future reordering flexibility. Similar to the settings.bin to settings.json migration, which renames settings.bin to settings.bin.bak, this change renames language.bin to language.bin.bak after adding the language setting to settings.json. --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**PARTIALLY**_ --- lib/I18n/I18n.cpp | 75 ++----------------- lib/I18n/I18n.h | 5 +- scripts/gen_i18n.py | 19 ++++- src/CrossPointSettings.cpp | 31 +++++++- src/CrossPointSettings.h | 3 + src/JsonSettingsIO.cpp | 9 +++ .../settings/LanguageSelectActivity.cpp | 8 +- src/main.cpp | 2 +- 8 files changed, 75 insertions(+), 77 deletions(-) diff --git a/lib/I18n/I18n.cpp b/lib/I18n/I18n.cpp index 00521df5..cf8d7449 100644 --- a/lib/I18n/I18n.cpp +++ b/lib/I18n/I18n.cpp @@ -1,19 +1,12 @@ #include "I18n.h" -#include -#include -#include - -#include +#include +#include #include "I18nStrings.h" using namespace i18n_strings; -// Settings file path -static constexpr const char* SETTINGS_FILE = "/.crosspoint/language.bin"; -static constexpr uint8_t SETTINGS_VERSION = 2; - I18n& I18n::getInstance() { static I18n instance; return instance; @@ -35,7 +28,6 @@ void I18n::setLanguage(Language lang) { return; } _language = lang; - saveSettings(); } const char* I18n::getLanguageName(Language lang) const { @@ -46,66 +38,11 @@ const char* I18n::getLanguageName(Language lang) const { return LANGUAGE_NAMES[index]; } -const char* I18n::getLanguageCode(Language lang) const { - const auto index = static_cast(lang); - if (index >= static_cast(Language::_COUNT)) { - return LANGUAGE_CODES[0]; - } - return LANGUAGE_CODES[index]; -} - -void I18n::saveSettings() { - Storage.mkdir("/.crosspoint"); - - FsFile file; - if (!Storage.openFileForWrite("I18N", SETTINGS_FILE, file)) { - LOG_ERR("I18N", "Failed to save settings"); - return; - } - - serialization::writePod(file, SETTINGS_VERSION); - - const char* code = getLanguageCode(_language); - serialization::writeString(file, code); - - LOG_DBG("I18N", "Settings saved: code=%s", code); -} - -void I18n::loadSettings() { - FsFile file; - if (!Storage.openFileForRead("I18N", SETTINGS_FILE, file)) { - LOG_DBG("I18N", "No settings file, using default"); - return; - } - - uint8_t version; - serialization::readPod(file, version); - - if (version == SETTINGS_VERSION) { - std::string code; - serialization::readString(file, code); - - for (uint8_t i = 0; i < getLanguageCount(); i++) { - if (code == LANGUAGE_CODES[i]) { - _language = static_cast(i); - LOG_DBG("I18N", "Loaded language: %s", code.c_str()); - return; - } - } - - LOG_ERR("I18N", "Unknown language code: %s", code.c_str()); - return; - } - - if (version == 1) { - uint8_t lang; - serialization::readPod(file, lang); - if (lang < static_cast(Language::_COUNT)) { - _language = static_cast(lang); - saveSettings(); - LOG_INF("I18N", "Migrated v1 language setting"); - } +Language I18n::languageFromCode(const char* code) { + for (uint8_t i = 0; i < getLanguageCount(); i++) { + if (strcmp(code, LANGUAGE_CODES[i]) == 0) return static_cast(i); } + return Language::EN; } // Generate character set for a specific language diff --git a/lib/I18n/I18n.h b/lib/I18n/I18n.h index 74273b30..fdbb9708 100644 --- a/lib/I18n/I18n.h +++ b/lib/I18n/I18n.h @@ -22,11 +22,8 @@ class I18n { Language getLanguage() const { return _language; } void setLanguage(Language lang); - const char* getLanguageCode(Language lang) const; const char* getLanguageName(Language lang) const; - - void saveSettings(); - void loadSettings(); + static Language languageFromCode(const char* code); // Get all unique characters used in a specific language // Returns a sorted string of unique characters diff --git a/scripts/gen_i18n.py b/scripts/gen_i18n.py index 520be52f..1d2eacb3 100755 --- a/scripts/gen_i18n.py +++ b/scripts/gen_i18n.py @@ -547,6 +547,23 @@ def generate_keys_header( "static_assert(sizeof(SORTED_LANGUAGE_INDICES) / sizeof(SORTED_LANGUAGE_INDICES[0]) == getLanguageCount()," ) lines.append(' "SORTED_LANGUAGE_INDICES size mismatch");') + lines.append("") + + # V1 language.bin migration table -- frozen enum order from commit 2f969a9. + # Maps the old uint8_t index stored on disk to the current Language enum. + # If a Language enum value listed here is ever removed, this will fail to + # compile, signalling that the migration table needs updating. + v1_codes = [ + "EN", "ES", "FR", "DE", "CS", "PT", "RU", "SV", "RO", "CA", "UK", + "BE", "IT", "PL", "FI", "DA", "NL", "TR", "KK", "HU", "LT", "SI", + ] + lines.append("// V1 language.bin migration table (frozen enum order from 2f969a9)") + lines.append("constexpr Language V1_LANGUAGES[] = {") + lines.append(" " + ", ".join(f"Language::{c}" for c in v1_codes) + ",") + lines.append("};") + lines.append( + f"constexpr uint8_t V1_LANGUAGE_COUNT = {len(v1_codes)};" + ) _write_file(output_path, lines, verbose) @@ -596,7 +613,7 @@ def generate_strings_cpp( "", ] - # LANGUAGE_NAMES array + # LANGUAGE_CODES array lines.append("// Language codes") lines.append("const char* const LANGUAGE_CODES[] = {") for code in languages: diff --git a/src/CrossPointSettings.cpp b/src/CrossPointSettings.cpp index b0ad9620..48d6d642 100644 --- a/src/CrossPointSettings.cpp +++ b/src/CrossPointSettings.cpp @@ -8,6 +8,7 @@ #include #include +#include "I18nKeys.h" #include "fontIds.h" // Initialize the static instance @@ -26,6 +27,8 @@ constexpr uint8_t SETTINGS_FILE_VERSION = 1; constexpr char SETTINGS_FILE_BIN[] = "/.crosspoint/settings.bin"; constexpr char SETTINGS_FILE_JSON[] = "/.crosspoint/settings.json"; constexpr char SETTINGS_FILE_BAK[] = "/.crosspoint/settings.bin.bak"; +constexpr char LANG_FILE_BIN[] = "/.crosspoint/language.bin"; +constexpr char LANG_FILE_BAK[] = "/.crosspoint/language.bin.bak"; // Convert legacy front button layout into explicit logical->hardware mapping. void applyLegacyFrontButtonLayout(CrossPointSettings& settings) { @@ -95,6 +98,7 @@ bool CrossPointSettings::loadFromFile() { LOG_ERR("CPS", "Failed to resave settings after format update"); } } + migrateLanguageBinaryFile(); return result; } } @@ -102,6 +106,7 @@ bool CrossPointSettings::loadFromFile() { // Fall back to binary migration if (Storage.exists(SETTINGS_FILE_BIN)) { if (loadFromBinaryFile()) { + migrateLanguageBinaryFile(); if (saveToFile()) { Storage.rename(SETTINGS_FILE_BIN, SETTINGS_FILE_BAK); LOG_DBG("CPS", "Migrated settings.bin to settings.json"); @@ -113,7 +118,31 @@ bool CrossPointSettings::loadFromFile() { } } - return false; + // No settings files at all -- check for standalone language.bin + return migrateLanguageBinaryFile(); +} + +bool CrossPointSettings::migrateLanguageBinaryFile() { + // V1_LANGUAGES / V1_LANGUAGE_COUNT are emitted by gen_i18n.py with the + // frozen enum order from 2f969a9. + if (!Storage.exists(LANG_FILE_BIN)) return false; + + FsFile f; + if (Storage.openFileForRead("CPS", LANG_FILE_BIN, f)) { + uint8_t version; + serialization::readPod(f, version); + if (version == 1) { + uint8_t oldIndex; + serialization::readPod(f, oldIndex); + if (oldIndex < V1_LANGUAGE_COUNT) { + language = static_cast(V1_LANGUAGES[oldIndex]); + } + } + } + Storage.rename(LANG_FILE_BIN, LANG_FILE_BAK); + saveToFile(); + LOG_DBG("CPS", "Migrated language.bin into settings.json"); + return true; } bool CrossPointSettings::loadFromBinaryFile() { diff --git a/src/CrossPointSettings.h b/src/CrossPointSettings.h index 5d35c9e9..1c58b73e 100644 --- a/src/CrossPointSettings.h +++ b/src/CrossPointSettings.h @@ -203,6 +203,8 @@ class CrossPointSettings { uint8_t imageRendering = IMAGES_DISPLAY; // Tilt-based page turning (X3 only — requires QMI8658 IMU) uint8_t tiltPageTurn = TILT_OFF; + // Language setting (Language enum index, default 0 = EN) + uint8_t language = 0; ~CrossPointSettings() = default; @@ -224,6 +226,7 @@ class CrossPointSettings { private: bool loadFromBinaryFile(); + bool migrateLanguageBinaryFile(); public: float getReaderLineCompression() const; diff --git a/src/JsonSettingsIO.cpp b/src/JsonSettingsIO.cpp index 487ac6f3..15b7d6b0 100644 --- a/src/JsonSettingsIO.cpp +++ b/src/JsonSettingsIO.cpp @@ -141,6 +141,10 @@ bool JsonSettingsIO::saveSettings(const CrossPointSettings& s, const char* path) doc["frontButtonLeft"] = s.frontButtonLeft; doc["frontButtonRight"] = s.frontButtonRight; + // Language -- managed by LanguageSelectActivity, not in SettingsList. + // Stored as ISO code string ("EN", "DE", ...) for stability across enum reorders. + doc["language"] = (s.language < getLanguageCount()) ? LANGUAGE_CODES[s.language] : "EN"; + String json; serializeJson(doc, json); return Storage.writeFile(path, json); @@ -220,6 +224,11 @@ bool JsonSettingsIO::loadSettings(CrossPointSettings& s, const char* json, bool* clamp(doc["frontButtonRight"] | (uint8_t)S::FRONT_HW_RIGHT, S::FRONT_BUTTON_HARDWARE_COUNT, S::FRONT_HW_RIGHT); CrossPointSettings::validateFrontButtonMapping(s); + // Language -- stored as code string for stability across enum reorders. + if (doc["language"].is()) { + s.language = static_cast(I18n::languageFromCode(doc["language"].as())); + } + LOG_DBG("CPS", "Settings loaded from file"); return true; diff --git a/src/activities/settings/LanguageSelectActivity.cpp b/src/activities/settings/LanguageSelectActivity.cpp index 5f21da26..ab5fd02c 100644 --- a/src/activities/settings/LanguageSelectActivity.cpp +++ b/src/activities/settings/LanguageSelectActivity.cpp @@ -6,6 +6,7 @@ #include #include +#include "CrossPointSettings.h" #include "I18nKeys.h" #include "MappedInputManager.h" #include "fontIds.h" @@ -49,11 +50,16 @@ void LanguageSelectActivity::loop() { } void LanguageSelectActivity::handleSelection() { + const uint8_t langIndex = SORTED_LANGUAGE_INDICES[selectedIndex]; + { RenderLock lock(*this); - I18N.setLanguage(static_cast(SORTED_LANGUAGE_INDICES[selectedIndex])); + I18N.setLanguage(static_cast(langIndex)); } + SETTINGS.language = langIndex; + SETTINGS.saveToFile(); + // Return to previous page onBack(); } diff --git a/src/main.cpp b/src/main.cpp index e2807816..8ff57d8b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -259,7 +259,7 @@ void setup() { HalSystem::checkPanic(); SETTINGS.loadFromFile(); - I18N.loadSettings(); + I18N.setLanguage(static_cast(SETTINGS.language)); KOREADER_STORE.loadFromFile(); OPDS_STORE.loadFromFile(); UITheme::getInstance().reload();