Suggestion for CHDK configuration file saving.

philmoz

3450

Re: Suggestion for CHDK configuration file saving.

« Reply #10 on: 03 / August / 2013, 22:09:38 »

Advertisements

Attached is a patch that implements the following:
- split config values into four files, core, OSD, user menu & GPS
- re-organise and group entries in the ConfInfo structures, all ID values redone (with gaps to allow new entries)
- add new tool to generate Lua libraries for each config file / ConfInfo structure (LUALIB/GEN/cnf_XXX.lua)
- add 'save_config_file' and 'load_config_file' calls to Lua
- general cleanup of old / unused stuff (e.g. ZOOM_OVERRIDE removed)

Note: the config files are all new, no attempt is made to convert previous config.

The Lua libraries can be used as per the following example, this turns off a bunch of OSD settings and saves a backup copy of the OSD config (the default OSD config file will get saved automatically):

Code: [Select]

--[[
@title Set OSD config
--]]

local osd=require("gen/cnf_osd")

set_config_autosave(0)

set_config_value(osd.splash_show, 0)
set_config_value(osd.space_icon_show, 0)
set_config_value(osd.space_perc_show, 0)
set_config_value(osd.space_mb_show, 0)
set_config_value(osd.space_bar_show, 0)
set_config_value(osd.show_temp, 0)
set_config_value(osd.show_clock, 0)
set_config_value(osd.batt_perc_show, 0)
set_config_value(osd.batt_volts_show, 0)
set_config_value(osd.batt_icon_show, 0)
set_config_value(osd.show_alt_helper, 0)

save_config_file(osd._config_id, "A/CHDK/OSD_BKUP.CFG")

Another example that loads the backup OSD config and forces saving to the default file:

Code: [Select]

--[[
@title Load OSD config
--]]

local osd=require("gen/cnf_osd")

load_config_file(osd._config_id, "A/CHDK/OSD_BKUP.CFG")
save_config_file(osd._config_id)

If the filename parameter to save_config_file / load_config_file is not supplied the default filename is used.
The first parameter defines which ConfInfo structure is to be saved/loaded.

In the latest patch (posted below) the old config ID values from 1.2 can be used in set_config_value and get_config_value:

Code: [Select]

set_config_value(47, 0)  -- maps to set_config_value(osd.splash_show, 0)

Things to look at / discuss:
- is there a better way to do the Lua libraries & code
- there is an opportunity here to rename config variables, some of the names are not entirely clear
- the split between core & OSD config may not be correct, there are some items that could be in either, I've made a choice for these; but open to suggestions for moving them

Phil.

« Last Edit: 05 / August / 2013, 06:21:55 by philmoz »

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)

msl

1280
A720 IS, SX220 HS 1.01a

Re: Suggestion for CHDK configuration file saving.

« Reply #11 on: 04 / August / 2013, 11:03:14 »

I like this new patch, great work.

But the patch has also consequences:
- Not all old scripts with set/get_config_value() will work.
- ptpCamGui needs an update. That means it is no more backward compatible.
- zeno's Config File Editor probably needs an Update.

msl

Logged

CHDK-DE: CHDK-DE links

philmoz

3450

Re: Suggestion for CHDK configuration file saving.

« Reply #12 on: 04 / August / 2013, 16:09:13 »

Quote from: msl on 04 / August / 2013, 11:03:14

I like this new patch, great work.

But the patch has also consequences:
- Not all old scripts with set/get_config_value() will work.
- ptpCamGui needs an update. That means it is no more backward compatible.
- zeno's Config File Editor probably needs an Update.

msl

I couldn't think of any clean way to maintain backward compatibility - keeping the old ID numbers meant that the ID values had to be unique across each ConfInfo structure. I felt this would create a maintenance nightmare in the future so chose to make a clean break with renumbering everything.

Any ideas on how it could be made more compatible would be appreciated.

One idea I did have was to have a non-zero base offset for all the Lua library values (core starts at 0 in the current patch), then add a lookup table for GetValue / SetValue for any id value < 1000 (old value) to convert to the new value. This would add a couple of K to the code size; but if breaking existing code is going to be a big problem then it might be worth doing.

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)

reyalp

14114

Re: Suggestion for CHDK configuration file saving.

« Reply #13 on: 04 / August / 2013, 16:28:00 »

First impressions:

This all looks pretty reasonable. Being able to refer to conf values by name is a big win for scripts.

One minor comment on the generated lua files: Mixing "administrative" values like config_id, first_entry, last_entry with actual values is somewhat awkward. It might be good to give them a set prefix (I suggest _ or __), so scripts can easily iterate over the actual conf values. You could put the ids in their own table, but that would make every reference require something like conf.ids.foo which is also awkward.

As far as compatibility goes:
This change would only be for 1.3. Scripts or tools that want to support both need to be version aware and do different things. However, since the 1.2 conf should not be changing much, there are some ways we could ease the process. My original plan was to wrap the generated conf modules with a higher level module like cap mode. It should be possible to do this in a way that works on both (a "generated" conf table for 1.2 would be required, but since that's basically frozen it could be built semi-manually)

It would also be possible to make module that wraps set_config_value with something that maps between the ids. This would be much better than doing the mapping in C code, IMO.

Logged

Don't forget what the H stands for.

reyalp

14114

Re: Suggestion for CHDK configuration file saving.

« Reply #14 on: 04 / August / 2013, 17:14:04 »

Quote from: philmoz on 03 / August / 2013, 22:09:38

- general cleanup of old / unused stuff (e.g. ZOOM_OVERRIDE removed)

Doing cleanup separately would make the relevant changes easier to follow, though I understand that you had to deal with some of this splitting up the conf stuff. I would not be averse to the cleanup part being checked in.

Some comments on conf saving. Not directly related to split, but relevant for 1.3
set_config_value will cause the conf (or all of them, in the new system) to be saved after every call. This isn't desirable if you are loading an entire config in a lua script.

I would suggest adding something like set_config_autosave(bool), so you could update all the conf values and then save once. When I was working on the ISO scripts, I ran into a case where setting the config values to close to shooting would run into the FsIoNotify assert. Note that turning autosave off would not make the changes temporary, since they could be autosaved later by the normal menu etc logic.

At some point it would be nice to get away from having a whole copy of the conf just to detect if it needs to be saved, but that's probably a project for another time.

Logged

Don't forget what the H stands for.

philmoz

3450

Re: Suggestion for CHDK configuration file saving.

« Reply #15 on: 04 / August / 2013, 18:33:11 »

Quote from: reyalp on 04 / August / 2013, 16:28:00

First impressions:

This all looks pretty reasonable. Being able to refer to conf values by name is a big win for scripts.

One minor comment on the generated lua files: Mixing "administrative" values like config_id, first_entry, last_entry with actual values is somewhat awkward. It might be good to give them a set prefix (I suggest _ or __), so scripts can easily iterate over the actual conf values. You could put the ids in their own table, but that would make every reference require something like conf.ids.foo which is also awkward.

Thanks, that's a good idea and an easy change.

Quote

As far as compatibility goes:

It would also be possible to make module that wraps set_config_value with something that maps between the ids. This would be much better than doing the mapping in C code, IMO.

I was thinking more about this some more, and it occurred to me that a mapping table in the C code would also allow loading of the CCHDK3.cfg file if the CCHDK4.cfg file was not found on the card. So it's probably worth doing for 1.3 - it can always be removed in 1.4

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: Suggestion for CHDK configuration file saving.

« Reply #16 on: 04 / August / 2013, 18:42:26 »

Quote from: reyalp on 04 / August / 2013, 17:14:04

Quote from: philmoz on 03 / August / 2013, 22:09:38
- general cleanup of old / unused stuff (e.g. ZOOM_OVERRIDE removed)
Doing cleanup separately would make the relevant changes easier to follow, though I understand that you had to deal with some of this splitting up the conf stuff. I would not be averse to the cleanup part being checked in.

Yes, I was going to commit those changes first (once I'm sure I haven't broken anything important).

Quote

Some comments on conf saving. Not directly related to split, but relevant for 1.3
set_config_value will cause the conf (or all of them, in the new system) to be saved after every call. This isn't desirable if you are loading an entire config in a lua script.

I would suggest adding something like set_config_autosave(bool), so you could update all the conf values and then save once. When I was working on the ISO scripts, I ran into a case where setting the config values to close to shooting would run into the FsIoNotify assert. Note that turning autosave off would not make the changes temporary, since they could be autosaved later by the normal menu etc logic.

I missed that, didn't realise it was saving after every update - definitely needs to be smarter.
Saving all the files every time is a kludge at the moment because the logic to detect changes is fairly dumb.

Quote

At some point it would be nice to get away from having a whole copy of the conf just to detect if it needs to be saved, but that's probably a project for another time.

Initially I was thinking of using a hash on the 'conf' structure instead of a copy & compare. Need to find a good hash algorithm that is fast; but will still detect all changes.

Just hashing or comparing the entire conf data doesn't help with determining which file(s) need to be updated. And comparing each value individually might get messy.

Forcing all the code to update conf values using modifier functions rather than directly might work; but would be a lot of code change.

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: Suggestion for CHDK configuration file saving.

« Reply #17 on: 05 / August / 2013, 06:19:02 »

New patch for testing.
- non config cleanup committed to SVN so no longer in this patch
- added map table to convert old ID's to new ID's for set_config_value & get_config_value
- map table used to load CCHDK3.CFG if no CCHDK4.CFG file found (converts 1.2 config to 1.3)
- add set_config_autosave function for Lua to turn off automatic saving in set_config_value
- added '_' to start of the 'admin' values in the Lua library files (e.g. config_id becomes _config_id).

Sample code in previous post updated.

Phil.

Edit: patch updated in later post.

« Last Edit: 09 / August / 2013, 23:05:14 by philmoz »

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)

msl

1280
A720 IS, SX220 HS 1.01a

Re: Suggestion for CHDK configuration file saving.

« Reply #18 on: 06 / August / 2013, 16:12:24 »

Really very good. Everything works as described. Also ptpCamGui runs fine with the old set/get_config_value().

I think this patch could be a first essential new feature in the CHDK version 1.3.

msl

Logged

CHDK-DE: CHDK-DE links

philmoz

3450

Re: Suggestion for CHDK configuration file saving.

« Reply #19 on: 07 / August / 2013, 04:33:56 »

Quote from: msl on 06 / August / 2013, 16:12:24

Really very good. Everything works as described. Also ptpCamGui runs fine with the old set/get_config_value().

I think this patch could be a first essential new feature in the CHDK version 1.3.

msl

One thing I forgot to mention is that in release 1.2, ID 270 is mapped to two different GPS config values (conf.gps_rec_play_set and conf.gps_track_symbol).

In the mapping table I set it to convert 270 to conf.gps_rec_play_set, since that was the one it would have found in the 1.2 code.

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)