CHDK UI version 2.0 ?

tsvstar

231

Re: CHDK UI version 2.0 ?

« Reply #230 on: 26 / July / 2012, 08:43:34 »

Advertisements

Phil,

Code doesn't support old format of paramset files. It use new format "var1=value1,var2=value2,...".
But it still parse header of scripts in same way

Not sure that this is good idea to support both format.

This a) will cancel most simplification benefits with inceased complexity back just to compatibility
b) I will need to detect format and choose different strategies, or just rollback load to previous code

And in same time:
a) param files will be moved to different directory, so you anyway you will have to do some portion of manual work if you would like to keep your params.
b) this is fresh 1.2. version. Just like with begining of flt version we shouldn't care about _strict_ back-compatibility. Too many things will be changed in this version
c) this is one-time migration to new version issue.

Actually saying as for me it is better to write short FLT module to migration if you think that this is really important thing to get old params. And keep core code pretty simple. There are other tasks which will increase its complexity exists. like human-readable paramsset name, temporary script run

« Last Edit: 26 / July / 2012, 08:47:40 by tsvstar »

Logged

philmoz

3450

Re: CHDK UI version 2.0 ?

« Reply #231 on: 26 / July / 2012, 16:53:00 »

Quote from: tsvstar on 26 / July / 2012, 08:43:34

Phil,

Code doesn't support old format of paramset files. It use new format "var1=value1,var2=value2,...".
But it still parse header of scripts in same way

Not sure that this is good idea to support both format.

This a) will cancel most simplification benefits with inceased complexity back just to compatibility
b) I will need to detect format and choose different strategies, or just rollback load to previous code

And in same time:
a) param files will be moved to different directory, so you anyway you will have to do some portion of manual work if you would like to keep your params.
b) this is fresh 1.2. version. Just like with begining of flt version we shouldn't care about _strict_ back-compatibility. Too many things will be changed in this version
c) this is one-time migration to new version issue.

Actually saying as for me it is better to write short FLT module to migration if you think that this is really important thing to get old params. And keep core code pretty simple. There are other tasks which will increase its complexity exists. like human-readable paramsset name, temporary script run

What does changing the format of the script parameter files achieve?

As you say the code to read the existing files is still there as it is also used to read the default parameter values from the script itself. So you aren't achieving any saving there.

For a major change that has the potential to break existing functionality for users, you need to ask yourself some questions:
- Does this change fix any existing bugs/issues/problems?
- Does the change result in significant functional improvements for users?
- Does the change give significant space savings or performance improvements?

If the answer to these questions is no, then is this a change you are doing simply because the existing code is not how you would have written it?

While we all have different ideas on how the CHDK code could be better, and we all suffer from the 'not invented here' syndrome from time to time, you have to consider the impact any change will have to the wider community.

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

philmoz

3450

Re: CHDK UI version 2.0 ?

« Reply #232 on: 26 / July / 2012, 18:44:08 »

Quote from: tsvstar on 26 / July / 2012, 08:43:34

It use new format "var1=value1,var2=value2,...".

Another thing to consider...

Currently the script parameters are numeric values only.
However it would not be difficult to add a method of allowing users to edit text string parameters using the text box module.

Now what happens if a user enters a text string with an embedded comma?

The current parameter file format will continue to work with only minor changes to the code.
But your format will break because of the embedded comma in the string - you will need to implement a complex parser to handle quoted strings, embedded commas etc.

Anyone who has tried to write code to parse arbitrary .CSV files will know how difficult this can be.

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

tsvstar

231

Re: CHDK UI version 2.0 ?

« Reply #233 on: 26 / July / 2012, 19:01:57 »

I clearly understand everything that you saying. I perfectly know this principles.

My reasons are:
1. I will use this new format in major improvement. This format will be used in cfg-driven menu.
2. Not only space saving but simplification could be the reason. Before big function process load and save such file. Now I just load this file and say "apply" (this function as I say required for futher cfg-driven menu). Save now also become pretty simple function.
3. This will simplify save/restore state of script to temporary run other (this improvement also should be done for same reason and after code simplification is pretty safe).

Is back-compatibility format so important? If yes, not a problem - simple "do_migrate" module could solve the problem.

PS.
And yes. I surely realize that it could make slightly more complex if strings will be implemented as input parameters. But even simple "\"-shielding could be solution for this problem.
I think that this problem should be solved if and when this will happens.

« Last Edit: 26 / July / 2012, 19:11:03 by tsvstar »

Logged

philmoz

3450

Re: CHDK UI version 2.0 ?

« Reply #234 on: 27 / July / 2012, 04:56:47 »

Revision 2014 in philmoz-uitest has all the latest trunk changes as well as the following:
- moved 'Restart Lua on error' setting to the script menu
- removed remote parameters menu from the script menu
- fixed some items on the ALT help screen for different variations of #define values

I've left the 'Disable Overrides' menu item text and options as is - I tried changing it to 'Enable Overrides'; but then the subsequent menu item, 'Include AutoIso & Bracketing?', did not make sense and I didn't want to change all the logic associated with that option as well.

If there are no issues I'll move all this to the trunk over the weekend.

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

waterwingz

12138

Re: CHDK UI version 2.0 ?

« Reply #235 on: 27 / July / 2012, 06:46:12 »

Quote from: philmoz on 27 / July / 2012, 04:56:47

I've left the 'Disable Overrides' menu item text and options as is - I tried changing it to 'Enable Overrides'; but then the subsequent menu item, 'Include AutoIso & Bracketing?', did not make sense and I didn't want to change all the logic associated with that option as well.

Don't have a camera loaded with your version with me this week. I think when we last discussed this, you had reduced the choices for "Disable Overrides" to no/yes ? (in conjuction with a better mechanism for managing shortcut overrides ?) If so, then is probably enough IMO.

Also, can you roll in this while you are doing the update : http://chdk.setepontos.com/index.php?topic=650.msg88362#msg88362 ?

Tnks

Logged

Ported : A1200 SD940 G10 Powershot N G16

msl

1280
A720 IS, SX220 HS 1.01a

Re: CHDK UI version 2.0 ?

« Reply #236 on: 27 / July / 2012, 06:59:29 »

Quote from: philmoz on 27 / July / 2012, 04:56:47

If there are no issues I'll move all this to the trunk over the weekend.

Quote from: waterwingz on 27 / July / 2012, 06:46:12

I think when we last discussed this, you had reduced the choices for "Disable Overrides" to no/yes ? (in conjuction with a better mechanism for managing shortcut overrides ?) If so, then is probably enough IMO.

I agree with that.

Hopefully we can also add the tsvstar's simple mode into the trunk. The simplification of the extra photo menu would be a great progress.

msl

Logged

CHDK-DE: CHDK-DE links

philmoz

3450

Re: CHDK UI version 2.0 ?

« Reply #237 on: 27 / July / 2012, 19:12:19 »

Quote from: waterwingz on 27 / July / 2012, 06:46:12

Also, can you roll in this while you are doing the update : http://chdk.setepontos.com/index.php?topic=650.msg88362#msg88362 ?

Can I suggest a couple of changes to this:
- move the 'added' message to inside the loop before the break statement
- change the message text to "'%s' added to User Menu" and fill in the name of the menu item to display in the message
- change the break to a return
- add a new message after the loop to tell the user that the 'User menu is full'

Phil.

Logged

CHDK ports:
sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
g12 (1.00c, 1.00e, 1.00f & 1.00g)
sx130is (1.01d & 1.01f)
ixus310hs (1.00a & 1.01a)
sx40hs (1.00d, 1.00g & 1.00i)
g1x (1.00e, 1.00f & 1.00g)
g5x (1.00c, 1.01a, 1.01b)
g7x2 (1.01a, 1.01b, 1.10b)

waterwingz

12138

Re: CHDK UI version 2.0 ?

« Reply #238 on: 27 / July / 2012, 19:54:54 »

Quote from: philmoz on 27 / July / 2012, 19:12:19

Can I suggest a couple of changes to this:

Of course - its a community project and I didn't spend more than 10 minutes on this change in the first place.

Quote

- move the 'added' message to inside the loop before the break statement

Agreed. I actually though of that afterward but couldn't decide if it was better to not acknowledge a "menu add" attempt if the menu is full (which I believe is the result of the change) or go a little farther and provide a "menu full" message. So I compromised on leaving it outside the loop.

Quote

- change the message text to "'%s' added to User Menu" and fill in the name of the menu item to display in the message

This is good too. Though about it earlier as well and decided it was a bit more code and that it might "overflow" the size of the message box for long menu entries ?

Quote

- change the break to a return
- add a new message after the loop to tell the user that the 'User menu is full'

Yup - this is good. Maybe I was a little too worried about saving a few bytes.

Will you just make the changes rather than me rolling up a new patch ? Should we "clamp" the menu entry part to some reasonable length ?

Thanks philmoz.

Logged

Ported : A1200 SD940 G10 Powershot N G16

tsvstar

231

Re: CHDK UI version 2.0 ?

« Reply #239 on: 27 / July / 2012, 23:22:09 »

I found the way how to keep param files on their places even for profiles. And so I see a possible sense to keep these files back-compatible.

But I also found one more reason to broke compatibility.

For now name of param file is made by
sprintf(ptr+shift-4, "_%d\0", param_set);

So for example paramset.lua will have param file paramset_0.
This is 10 symbol per name, while 8 is actually recommended limitation.
Filebrowser do not display this correct. Possible some other functionality (including futher) could have problem caused by this.

Correct solution for this should be something like
"._%c%d",extension[0],param_set
This will: a) make strict format correspondance
b) separate same named but different ext scripts

But this still require to broke paramfile bak-compatibility.
BTW: CHDK.CFG is renamed from release to release. And this surely much more broke inter-version compatibility. Isn't it?

Logged