supplierdeeply

Suggestion for CHDK configuration file saving.

  • 38 Replies
  • 2783 Views
*

Online philmoz

  • *****
  • 2936
    • Photos
  • Publish
    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 »
    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)

    *

    Offline msl

    • *****
    • 1172
    • A720 IS, SX220 HS 1.01a
      • CHDK inside
  • Publish
    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
    German CHDK pages:  CHDK forum | CHDK inside | CHDK Twitter News by msl | Download CHDK-DE (Autobuild)
    Note: SDM violates the GPL rules!

    *

    Online philmoz

    • *****
    • 2936
      • Photos
  • Publish
    Re: Suggestion for CHDK configuration file saving.
    « Reply #12 on: 04 / August / 2013, 16:09:13 »
    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.
    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)

    *

    Offline reyalp

    • ******
    • 9801
  • Publish
    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.
    Don't forget what the H stands for.


    *

    Offline reyalp

    • ******
    • 9801
  • Publish
    Re: Suggestion for CHDK configuration file saving.
    « Reply #14 on: 04 / August / 2013, 17:14:04 »
    - 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.
    Don't forget what the H stands for.

    *

    Online philmoz

    • *****
    • 2936
      • Photos
  • Publish
    Re: Suggestion for CHDK configuration file saving.
    « Reply #15 on: 04 / August / 2013, 18:33:11 »
    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.
    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)

    *

    Online philmoz

    • *****
    • 2936
      • Photos
  • Publish
    Re: Suggestion for CHDK configuration file saving.
    « Reply #16 on: 04 / August / 2013, 18:42:26 »
    - 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.
    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)

    *

    Online philmoz

    • *****
    • 2936
      • Photos
  • Publish
    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 »
    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)


    *

    Offline msl

    • *****
    • 1172
    • A720 IS, SX220 HS 1.01a
      • CHDK inside
  • Publish
    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
    German CHDK pages:  CHDK forum | CHDK inside | CHDK Twitter News by msl | Download CHDK-DE (Autobuild)
    Note: SDM violates the GPL rules!

    *

    Online philmoz

    • *****
    • 2936
      • Photos
  • Publish
    Re: Suggestion for CHDK configuration file saving.
    « Reply #19 on: 07 / August / 2013, 04:33:56 »
    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.
    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)

     

    Related Topics