Skip to content
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

Support simplified chinese #452

Merged
merged 22 commits into from
Nov 16, 2024
Merged

Conversation

ligenxxxx
Copy link
Member

@ligenxxxx ligenxxxx commented Nov 6, 2024

Considering that some users have difficulty reading English, I decided not to add a language option in the menu.
We use the SD card method to change the language.

  • Create an empty file in the root directory of the SD card named:
    -- CHN.TXT or chn.txt mean Chinese.
    -- ENG.TXT or eng.txt mean English.
  • Insert the SD card into Goggle and restart, Goggle will be set.

The lvgl private font library was removed by mistake and needs to be repaired.
@ligenxxxx ligenxxxx marked this pull request as ready for review November 13, 2024 06:53
@Master92
Copy link
Contributor

Master92 commented Nov 14, 2024

Awesome! I've got some suggestions for performance and future l10ns:

  1. Extract translations into .ini files. We already use minIni and could also use that library to parse the file at bootup/runtime.
    1. I just looked into the minIni implementation and they don't cache the file. So we would probably have to manually parse the whole file and cache it ourselves or use a caching implementation. I've implemented an ini parser myself but currently it only supports C++20 and up so I would have to port it to make it work: https://github.com/Master92/cppIni
  2. Use a key-value approach for querying the localized strings. In the first step we could use a tree structure if we want to strictly adhere to C or make use of the compiler's capability and use an std::map under the hood.
  3. Alternatively add an early-return to the translate_string method for when the requested language is English as early as possible.

Another thing I don't understand is why do you search for an integer match in language.c? You can use the argument lang and compare that for less than LANG_END and use it for accessing the array after that was succesfull. Also, if for some reason a value greater than LANG_END is passed to the method, you currently have an endless loop as i won't get increased and never satisfies the breaking condition of the loop in line 22.

You should also change the type of translate_t's members to const char * const and translate_string to const char*. Otherwise, anything can happen to the translations at runtime as the consumers are allowed to change them.

I'd also suggest adding a CMake target for generating and copying the Chinese font file (and possibly more to come) automatically before compiling (if necessary). I can help with that if you like and give me write access to your branch 😉

@ligenxxxx
Copy link
Member Author

  • I think using .ini is a good idea but don't want to finish it in this PR. You can create a new PR after I merge this PR.
  • I don't want to add font generation in cmake. It requires installing an additional environment. And generating fonts is not that fast. I just don't think it's necessary.

@Master92
Copy link
Contributor

  • I think using .ini is a good idea but don't want to finish it in this PR. You can create a new PR after I merge this PR.

Fair enough. I just read through the manual of minIni and found that there is a way to accomplish what I was intending concerning caching and not having to read the file over and over again. I'll start with that as soon as this PR has been merged 👍

  • I don't want to add font generation in cmake. It requires installing an additional environment. And generating fonts is not that fast. I just don't think it's necessary.

Alright, I overlooked the necessity to npm install the conversion package and all. You're right, that's not really feasable for a CMake target.

@ligenxxxx ligenxxxx merged commit e947634 into hd-zero:main Nov 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants