PB, request for code review and merge back(2)

vlad_vy
Offline

12 Nov 2012 04:58 - 12 Nov 2012 05:00 #2865 by vlad_vy

Replied by vlad_vy on topic PB, request for code review and merge back(2)

In my liking Devo8 'Tx Configure' with scrolling is not obvious. Will be better to have 2 pages, for example 'Tx Configure' and 'Telemetry Units'.

Model 'Telemetry' page wil be better name 'Telemetry Alarms', at top of the page 'Alarms' and simply numbers of alarms (1, 2, 3, 4, 5, 6). Plus, add control for type of alarm (for example: Once, 1s, 2s, 3s, ...). Will be nice to have different sounds for each alarm, by default.

Last edit: 12 Nov 2012 05:00 by vlad_vy.

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

12 Nov 2012 05:00 #2866 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

Actually, I prefer putting telemtry config back to telemetry monitor pages.
Though devo10 should have exactly the same functionalities as devo8, they could have different UI presentation. And in devo10, add more menu items is better than adding more items into a configure pages, in order to get to expected items as quick as possible

No, it definitely makes much more sense where it is now. It is a transmitter property. It should not be set for each model, but once for each transmitter.[/quote]

In previous design, the telemetry unit's configure is still under tx configure sub-menu , but it was put together with the Telemetry monitor page. By this way, it is more nature for users to be change telem units when they monitor telem data.
And I believe the "tx - basic configure" and "model setup" should be a place to put generic or misc items
Currently I am working on adding timer alarm config, I would rather put them in the same page with the timer instead of in the page of the model setup

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

12 Nov 2012 13:50 #2869 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

Submitted a new feature and several bug fixes, please check them out
1) Allow users to customize countdown timer's alarm settings: beep 1 for every given interval1 when countdown time <= given prealert time; beep 1 for every given interval2 when coutndown time <0
2) Fix a major bug in devo8: when click the chantest button in the mixer page, all limits are reset to 0
3) Fix a minor bug in devo10: safety value won't be saved if the safety is not changed
4) Improve devo10' tx_configure page : showing group titles in the page
5) Provide heli.bmp and plane.bmp in devo10's filesystem
6) Increase the MAX_STRINGS in language.c

Please Log in or Create an account to join the conversation.

PhracturedBlue
Offline

12 Nov 2012 14:13 #2872 by PhracturedBlue

Replied by PhracturedBlue on topic PB, request for code review and merge back(2)

suvsuv wrote: Submitted a new feature and several bug fixes, please check them out
1) Allow users to customize countdown timer's alarm settings: beep 1 for every given interval1 when countdown time <= given prealert time; beep 1 for every given interval2 when coutndown time <0
2) Fix a major bug in devo8: when click the chantest button in the mixer page, all limits are reset to 0
3) Fix a minor bug in devo10: safety value won't be saved if the safety is not changed
4) Improve devo10' tx_configure page : showing group titles in the page
5) Provide heli.bmp and plane.bmp in devo10's filesystem
6) Increase the MAX_STRINGS in language.c

Thanks. I like the layout of the tx configuration.
I'll go through the code and start the merge, but at 1st glance everything looks ok.

Please Log in or Create an account to join the conversation.

PhracturedBlue
Offline

12 Nov 2012 14:17 #2874 by PhracturedBlue

Replied by PhracturedBlue on topic PB, request for code review and merge back(2)

for #6, I'd like to find a better way. maybe I can look at the output list and filter the language files per model (so we maintain one language file, but it gets split up for each Tx). Otherwise the language will just continue to take up more and more RAM as we go forward.
Another option is just to filter based on strings in the different page dirs. That may be easier to do reliably.

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

12 Nov 2012 14:29 #2875 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

PhracturedBlue wrote: for #6, I'd like to find a better way. maybe I can look at the output list and filter the language files per model (so we maintain one language file, but it gets split up for each Tx). Otherwise the language will just continue to take up more and more RAM as we go forward.
Another option is just to filter based on strings in the different page dirs. That may be easier to do reliably.

Sure, I am looking forward to your solution.
Currently, having 1 lang file for 2 types of TXes makes the lang string's size keep growing. However, letting the 2 types of TXes use exactly the same string sets is really hard due to devo10's LCD constraint.

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

13 Nov 2012 03:45 #2886 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

PB, I have my repo synced with your repo and just saw 1 bug fix.
I made the following minor changes, all related to langs
1) Update lang.cn, finish tranlation for Simplified Chinese
2) Adjust some items' width in devo10
3) Have lang strings coincided as more as possible for devo8 and devo10
4) Move the USB and About to main menu in devo10

So far, there should be not any UI changes remaining. I suggest to prepare a stable release for both devo8 and devo10

Please Log in or Create an account to join the conversation.

PhracturedBlue
Offline

13 Nov 2012 04:44 #2890 by PhracturedBlue

Replied by PhracturedBlue on topic PB, request for code review and merge back(2)

I think it would make more sense for the telemetry and usb to be on the main page than about and usb. It should be quick to get to the telemetry page, and unlike the devo8, you can't get there with a touch. wel could have all 3, but that will, I think require scrolling which I'm not sure adds any value.

Fyi, before modifying the language files you should always run:
make language
This puts the strings in order and will identify all of the misisng strings and remove unused ones.

Please Log in or Create an account to join the conversation.

FDR
Offline

13 Nov 2012 05:31 - 13 Nov 2012 05:33 #2892 by FDR

Replied by FDR on topic PB, request for code review and merge back(2)

...or for one language:

../utils/extract_strings.pl -lang hu -update

...where you can replace "hu" with the desired language extension...

Last edit: 13 Nov 2012 05:33 by FDR.

Please Log in or Create an account to join the conversation.

vlad_vy
Offline

13 Nov 2012 06:31 #2893 by vlad_vy

Replied by vlad_vy on topic PB, request for code review and merge back(2)

I don't like current Devo8 Tx 'Configure' page. At russain interface many strings don't fit, with very shortened russian strings and removed 'telemetry' words(anyway strings don't fit) all become absolutely unclear.

At my opinion, will be better return 'Backlight', 'Dimmer time', 'Dimmer target' and 'Battery alarm' to upper part of page. Then at lower part of page divide telemetry and timer settings by some space and add headers 'Telemetry Units' and 'Timer Settings'.

And anyway, every time, at first I press 'Next' page button, then return to 'Configure' page and scroll down.

Please Log in or Create an account to join the conversation.

vlad_vy
Offline

13 Nov 2012 06:41 #2894 by vlad_vy

Replied by vlad_vy on topic PB, request for code review and merge back(2)

Also, seems that russian language file again is too large. At complex Mixer 'Mux:' items is not translated.

Please Log in or Create an account to join the conversation.

PhracturedBlue
Offline

13 Nov 2012 06:52 #2895 by PhracturedBlue

Replied by PhracturedBlue on topic PB, request for code review and merge back(2)

vlad_vy wrote: I don't like current Devo8 Tx 'Configure' page. At russain interface many strings don't fit, with very shortened russian strings and removed 'telemetry' words(anyway strings don't fit) all become absolutely unclear.

Yes, we all agree it needs to be better. I though FDR was going to take a stab at it, but if not, I'll do it myself. Maybe like the devo10 page which is pretty well organized I think.

Also, seems that russian language file again is too large. At complex Mixer 'Mux:' items is not translated.

I think it is ok. However, I changed the build process. The Makefile now converts the lang.* files in common/langauge to only include the necessary strings in devo*/language. this means the language files in common, devo8, and devo10 are all different (with common being the superset).
If you load the language files from the proper dir, it should work. Well, at least it works fine for me.
If you look at the filesizes, you should see something like:
8240 2012-11-12 22:35 filesystem/common/language/lang.ru
6093 2012-11-12 22:45 filesystem/devo10/language/lang.ru
6719 2012-11-12 22:45 filesystem/devo8/language/lang.ru

Anyhow, I went through all the menus in russian and didn't see any issues.

Please Log in or Create an account to join the conversation.

vlad_vy
Offline

13 Nov 2012 07:09 - 13 Nov 2012 07:12 #2896 by vlad_vy

Replied by vlad_vy on topic PB, request for code review and merge back(2)

Thanks. With latest build russian translation looks fine. Possibly I place corrected russian file to filesystem/devo8/language/lang.ru

Last edit: 13 Nov 2012 07:12 by vlad_vy.

Please Log in or Create an account to join the conversation.

xCometz
Offline

13 Nov 2012 07:26 #2897 by xCometz

Replied by xCometz on topic PB, request for code review and merge back(2)

suvsuv wrote: Run a quick test to measure current consumption between LCD standby/sleep and normal more, entering standby mode can only save less than 1mA when backlight is off, I don't think it is a very meaningful feature to be implemented in devo10.

Suvsuv, did you disable all LCD access activities while in sleep mode?
Please, try to disable GUI_RefreshScreen() in sleep mode and measure the current once again. Did you measure current on battery supply or 3V3?

Please Log in or Create an account to join the conversation.

vlad_vy
Offline

13 Nov 2012 07:32 #2898 by vlad_vy

Replied by vlad_vy on topic PB, request for code review and merge back(2)

vlad_vy wrote: And anyway, every time, at first I press 'Next' page button, then return to 'Configure' page and scroll down.

I think that I do so because there is many free space at first part of Tx 'Configure' page that doesn't imply any continuation. Any new user will do the same.

Please Log in or Create an account to join the conversation.

sbstnp
Offline

13 Nov 2012 08:13 #2899 by sbstnp

Replied by sbstnp on topic PB, request for code review and merge back(2)

PhracturedBlue wrote: I think it would make more sense for the telemetry and usb to be on the main page than about and usb. It should be quick to get to the telemetry page, and unlike the devo8, you can't get there with a touch.

IMO telemetry should stay where it is currently, not every model we fly has telemetry, and if we want quick acces, we could implement a shortcut system and bind the page to the R or L keys.

USB and About pages make more sense in the main menu with them being generic items not tied to a feature of a model/rx, in this case telemetry. Probably we could enable/disable telemetry on a per model basis (either manually or automatically if the protocol allows) and show/hide the menu entry. That would be best from a UX perspective.

But in the end I since nobody commented on the issue I raised on bitbucket I guess nobody cares enough, so whatever flies your model

. I'll probably just patch the code the way I like it.

Devo 10 + 4in1
~~Spektrum Dx9~~
FrSky Taranis + TBS Crossfire

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

13 Nov 2012 08:13 - 13 Nov 2012 09:01 #2900 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

PhracturedBlue wrote: I think it would make more sense for the telemetry and usb to be on the main page than about and usb. It should be quick to get to the telemetry page, and unlike the devo8, you can't get there with a touch. wel could have all 3, but that will, I think require scrolling which I'm not sure adds any value.

Fyi, before modifying the language files you should always run:
make language
This puts the strings in order and will identify all of the misisng strings and remove unused ones.

My repo is all in sync with update now, except for the lang.cn and lang.tw file.
Currently, the string number in a lang file exceeds the 256 limit

Last edit: 13 Nov 2012 09:01 by suvsuv.

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

13 Nov 2012 11:47 #2902 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

xCometz wrote:
suvsuv wrote: Run a quick test to measure current consumption between LCD standby/sleep and normal more, entering standby mode can only save less than 1mA when backlight is off, I don't think it is a very meaningful feature to be implemented in devo10.

Suvsuv, did you disable all LCD access activities while in sleep mode?
Please, try to disable GUI_RefreshScreen() in sleep mode and measure the current once again. Did you measure current on battery supply or 3V3?

The current is measured on battery supply. By disabling GUI_RefreshScreen() during idle, I don't see any improvement on current consumption compared to yesterday's test result . Putting LCD to standby turns out to be useless when backlight is off

Please Log in or Create an account to join the conversation.

suvsuv
Topic Author
Offline

13 Nov 2012 12:00 #2903 by suvsuv

Replied by suvsuv on topic PB, request for code review and merge back(2)

PB, please remove generating lang strings automatically from the makefile, it is annoying and might mask compile warnings. I prefer to type "make language" manually.

Please Log in or Create an account to join the conversation.

FDR
Offline

13 Nov 2012 12:16 #2904 by FDR

Replied by FDR on topic PB, request for code review and merge back(2)

PhracturedBlue wrote:
vlad_vy wrote: I don't like current Devo8 Tx 'Configure' page. At russain interface many strings don't fit, with very shortened russian strings and removed 'telemetry' words(anyway strings don't fit) all become absolutely unclear.
Yes, we all agree it needs to be better. I though FDR was going to take a stab at it, but if not, I'll do it myself. Maybe like the devo10 page which is pretty well organized I think.

Sorry, I had no time at all for it, not even for updating the language file.
Besides, you have discouraged me about the placements...

Please Log in or Create an account to join the conversation.

Time to create page: 0.075 seconds

Force desktop display