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

[WIP] Add translation string support #10995

Closed
wants to merge 5 commits into from
Closed

[WIP] Add translation string support #10995

wants to merge 5 commits into from

Conversation

hamishwillee
Copy link
Contributor

@hamishwillee hamishwillee commented Dec 6, 2018

@dagar This is my first shot at adding translation string support. Gus still needs to add support for the format described in part 3. This is apparently easy as it is a direct copy of what he does for the camera definition files.

It has three main parts:

  1. Generation of parameter_strings.xml containing map of keys to English strings.
  • Created by a new option --translation_xml in *px_process_params.py
  • This is called in the cmakelists to copy new file to parameters/translations/parameter_strings.xml
  • Format looks like below. Has unique key based on param name and subinfo - ie enumvalue, bit index, short/long description. Groups and Categories are "Bare - ie no special key because they have no sub items. We could add group as a parent in parameter index but decided not to.
<?xml version='1.0' encoding='UTF-8'?>
<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
<properties>
<entry key="Attitude Q estimator">Attitude Q estimator</entry>
<entry key="ATT_ACC_COMP_short_desc">Acceleration compensation based on GPS velocity</entry>
<entry key="ATT_EXT_HDG_M_enumval_1">Vision</entry>
<entry key="EKF2_AID_MASK_bit_5">multi-rotor drag fusion</entry>
</properties>
  1. Crowdin integration
  • We can set up crowdin to monitor Firmware/translations/parameter_strings.xml in github and spit out translated files with format Firmware/translations/parameter_strings-xx_YY.xml (e.g. zh_CH) in the same directory.
  1. Addition of localisation to the parameters.xml
  • We add a localization to end of parameters that has locale tags of following format for each locale.
...
</group>
<localization>
<locale name="zh_CN">
<strings original="Vision" translated="视觉训练" />
<strings original="Motion Capture" translated="运动捕捉" />
<strings original="Complimentary filter external heading weight" translated="互补过滤器外部航向权重值" />
<strings original="Acceleration compensation based on GPS&#10;velocity" translated="基于 GPS 的加速度补偿&#10;速度" />
<strings original="Complimentary filter magnetometer weight" translated="互补滤波器磁罗盘权重值" />
<strings original="Attitude Q estimator" translated="姿态 Q 估计器" />
<strings original="Battery Calibration" translated="电池校准" />
<strings original="Magnetic declination, in degrees" translated="磁偏角, 以角度为单位" />
<strings original="Gyro bias limit" translated="陀螺偏置极限值" />
</locale>
</localization>
</parameters>
  • The code parses Firmware/translations and generates a dictionary of language dictionaries. The translations dict ONLY have translated terms.
  • This is passed into the xmlout to generate the locale information.

This will not fall over if the translation files are missing.

Once this (or something like it) is agreed we can add the crowdin integration.

Without keys tidy - probably going to revert

Updates to param scripts

Move translation scanning into own module
@hamishwillee hamishwillee requested a review from dagar December 6, 2018 06:04
@hamishwillee
Copy link
Contributor Author

PS I am fully expecting you to hate my Python. That can be tidied if the overall approach is considered OK.

@bys1123
Copy link
Contributor

bys1123 commented Dec 6, 2018

Thanks for your work, we will join translation when it's ready.

@hamishwillee
Copy link
Contributor Author

@dagar The build now passes - the failure appears unrelated to this.

Can you please ETA review?

CMakeLists.txt Outdated Show resolved Hide resolved
@dagar
Copy link
Member

dagar commented Dec 11, 2018

We should add something simple to CI (Jenkins) that runs this. Next we'd have it post to Crowdin automatically on an overall successful master build.

Example Generate Metadata -
https://github.com/PX4/Firmware/blob/master/Jenkinsfile#L291-L331
Runs on Jenkins here - http://ci.px4.io:8080/blue/organizations/jenkins/PX4%2FFirmware/detail/master/440/pipeline

@hamishwillee
Copy link
Contributor Author

We should add something simple to CI (Jenkins) that runs this.

Yes.

Next we'd have it post to Crowdin automatically on an overall successful master build.

I don't think we want to do this. Instead we want to automatically save /src/lib/parameters/translations/parameter_strings.xml into the github tree. Crowdin can be set up to automatically suck out updates and export a PR to spit out the translated results into the same directly.

@bys1123
Copy link
Contributor

bys1123 commented Dec 28, 2018

We tried to run make parameters_metadata, looks like few long_desc is missing in parameter_strings.xml.
eg. WEST_BETA_GATE, WEST_TAS_GATE
Is this already clean up duplicated strings?
@hamishwillee


for group in groups:
group_name=group.GetName()
if group_name not in unique_strings:
Copy link
Contributor

@bys1123 bys1123 Dec 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, looks like it is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed :-)

This is waiting on @dagar - he will review and merge (if appropriate) when it gets near the top of his priority list.

@stale
Copy link

stale bot commented Jul 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@bys1123
Copy link
Contributor

bys1123 commented Jul 11, 2019

(っ•̀ω•́)っ✎⁾⁾

@stale stale bot removed the Admin: Wont fix label Jul 11, 2019
@hamishwillee
Copy link
Contributor Author

@bkueng FYI, this is languishing, but the problem of parameter translation is worth thinking about when you look at the broader "strings in PX4" translation support.

@stale
Copy link

stale bot commented Oct 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@bys1123
Copy link
Contributor

bys1123 commented Mar 11, 2020

@hamishwillee Hey, What is the stage of this PR now?

@stale stale bot removed the stale label Mar 11, 2020
@hamishwillee
Copy link
Contributor Author

@bys1123 Nowhere.
I think that this will be addressed in part by @bkueng work in the events interface.
But this PR is now so out of sync we'd have to start again to merge it. I'm therefore closing.

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.

3 participants