new branch - CHDK : Elf Edition - Developers wanted - page 20 - General Discussion and Assistance - CHDK Forum supplierdeeply
« previous next »
Pages: 1 ... 15 16 17 18 19 [20] 21 22 23 24 25 ... 32   Go Down

new branch - CHDK : Elf Edition - Developers wanted

  • 316 Replies
  • 117693 Views
*

Offline outslider

  • *****
  • 762
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #190 on: 07 / January / 2012, 06:36:44 »
Advertisements
Quote
We could ask outslider does this fix help.

But I don't really understand, what the fix is;) I don't know almost anything about modular code and the place of the code you have posted ;)

BTW - other problem - pressing shoot_full while any module is working causes script start/stop. I believe that this is not intended. Maybe there could be similar fix as was used before for fselect?
http://trac.assembla.com/chdk/changeset/1461
« Last Edit: 07 / January / 2012, 18:13:13 by outslider »
Logged
if (2*b || !2*b) {
    cout<<question
}

Compile error: poor Yorick
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #191 on: 08 / January / 2012, 07:17:12 »
Changeset 1534 in the reyalp-flt branch has the following changes to the module system for review.

- support for modules to include menus, module can be optionally unloaded when menu exits
- moved the edge overlay menu to the edgeovr module
- moved the curves menu to the curves module
- moved bitvector to edge overlay module (only place it is used)
- moved zebra code to seperate files (zebra.c & zebra.h) and converted to module (with menus)
- converted grid code to a module (with menus)
- converted motion detect code to a module
- added the 'conf' structure (versioned) to the export list and removed CONF_BIND_xxx macros. The conf structure is heavily used in menus.
- cleaned up the modules.h file (moved sections to the individual module header files)
- cleaned up the code in modules.c
- fixed spelling mistakes
- removed un-necesary header files for games
- replaced OPT_xxx compile options for individual games with a single OPT_GAMES option (undef to remove 'Games' menu).
- updated DNG code to not load dng module unless required
- restored the static menu structure to match the 1.0 release (the module inspector replaces the Modules sub-menu)
- removed the dynamic module directory scan and menu creation at startup (due to current crash problem)
- re-ordered the modules_exportlist.c array of exported symbols

In addition there is a small change to add macros for managing the palette colors. This is the first part of re-work for palette system that I was doing when I got sidetracked on the module/menu/crash problem.

This is work-in-progress, although I've tested it there may be things I've missed.

Feedback appreciated.

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)
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #192 on: 08 / January / 2012, 18:30:02 »
Unfortunately I have not enought time right now to precise look to changes but on quick look I have few questions:

1. Why export_list is rearranged greatly? It should never be touched. Because it absoultely broke module compataibility. Moreover broke it silently. I see no reason for so big changes. New entries should be added to the end.
And as I described somewhere in the middle of thread - noone should worry about logically grouping in any way this list just for logical grouping. This grouping will be broken surely again in future because new entries will be added to the end. And what symbols require to be added to the export_list will be sayed by elf2flt automatically.

2. I do not understand why MODULEINFO_FLAG_SYSTEM added everywhere?
Purpose of this flag is hiding module in module_menu. It should be used for two kind of modules and nothing more.
a) Library modules. This modules are strongly related to CHDK and can't be used in seprately.
b) System modules such as mpopup, module_menu or futher textbox. Such  modules are actually auxilary part of system and have no value as standalone module. They have value in context of they are ran. Moreover, they have to be run with parameters only.

Any modules which could be used standalone shouldn't have this flag raised. Even if this module have binding to static menu item, IMO. This is user preference use static or dynamic linking.
If this flag raised, I have only very unfriendly way to run it on different CHDK (previous revision for example - run it from filebrowser).

3. I think that static binding should co-exist with dynamic. My personal preference is dynamic menu even with not resolved yet stability problem (for now just require more patience to start menu from my. I reach calm and dzen :)). BTW It is not called and so doesn't crash on startup - this module is called on first menu call. So even though this is unstable, stable way how to start exists (just wait while camera started)
 So IMO this should be configurable in OPT_ at least

PS - nice generalizations of module. I had no time for that. :)

PS2 - I do not check but module unloading is potentialy unsafe theme. That's why not all and not always libraries are unloaded. Just remind because can't take a look.
« Last Edit: 08 / January / 2012, 18:38:33 by tsvstar »
Logged
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #193 on: 08 / January / 2012, 20:41:29 »
Thanks for the feedback,

Quote from: tsvstar on 08 / January / 2012, 18:30:02
1. Why export_list is rearranged greatly? It should never be touched. Because it absoultely broke module compataibility. Moreover broke it silently. I see no reason for so big changes. New entries should be added to the end.
And as I described somewhere in the middle of thread - noone should worry about logically grouping in any way this list just for logical grouping. This grouping will be broken surely again in future because new entries will be added to the end. And what symbols require to be added to the export_list will be sayed by elf2flt automatically.

This was so I could get a better understanding of the functional areas being exported.
I wanted to look for:
- places where it makes sense to add exported functions now since they may be needed in future (e.g. the draw_xxx functions)
- areas where it might make more sense to add a versioned API struct of function pointers instead, where the interface may change (e.g. the gui and viewport functions).

In my view it is more important to get a good foundation in place rather than worry about absolute module compatability (that will come of course).

(I also removed a number of entries, such as the bitvector functions, so the compatibility was going to be broken regardless.)

Quote
2. I do not understand why MODULEINFO_FLAG_SYSTEM added everywhere?
Purpose of this flag is hiding module in module_menu. It should be used for two kind of modules and nothing more.
a) Library modules. This modules are strongly related to CHDK and can't be used in seprately.
b) System modules such as mpopup, module_menu or futher textbox. Such  modules are actually auxilary part of system and have no value as standalone module. They have value in context of they are ran. Moreover, they have to be run with parameters only.

Any modules which could be used standalone shouldn't have this flag raised. Even if this module have binding to static menu item, IMO. This is user preference use static or dynamic linking.
If this flag raised, I have only very unfriendly way to run it on different CHDK (previous revision for example - run it from filebrowser).

I was using this as I moved modules into the static menus so I could see what was left in the dynamic list - I will restore the correct settings.

Quote
3. I think that static binding should co-exist with dynamic. My personal preference is dynamic menu even with not resolved yet stability problem (for now just require more patience to start menu from my. I reach calm and dzen :)). BTW It is not called and so doesn't crash on startup - this module is called on first menu call. So even though this is unstable, stable way how to start exists (just wait while camera started)
 So IMO this should be configurable in OPT_ at least

I think this area needs more thought and investigation, especially until there is a better solution for the crash problem. For example a config file of modules to load into a 'Modules' menu could be an interim approach that would avoid the 'Opendir' crash issue and would still provide flexibility to add new modules without needing static menu entries..

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)
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #194 on: 09 / January / 2012, 01:25:12 »
Quote from: philmoz on 08 / January / 2012, 20:41:29
This was so I could get a better understanding of the functional areas being exported.
I wanted to look for:
- places where it makes sense to add exported functions now since they may be needed in future (e.g. the draw_xxx functions)
- areas where it might make more sense to add a versioned API struct of function pointers instead, where the interface may change (e.g. the gui and viewport functions).

In my view it is more important to get a good foundation in place rather than worry about absolute module compatability (that will come of course).

(I also removed a number of entries, such as the bitvector functions, so the compatibility was going to be broken regardless.)
I do agree with you that good foundation on this stage is more important thing.

Just one more note about one of case of changes in export_list. If some exported functions are removed from CHDK in any reason, then correspondent record in export_list should be changed to 0. This both keep compatibility and safety (modules which use such symbols just will not be loaded)

Quote
I think this area needs more thought and investigation, especially until there is a better solution for the crash problem. For example a config file of modules to load into a 'Modules' menu could be an interim approach that would avoid the 'Opendir' crash issue and would still provide flexibility to add new modules without needing static menu entries..
This was one of first idea how to implement this module. But I prefer from my experience the way which require nothing from user at all.

Few different ideas:
1. Use text config, but integrate this with futher planed module which should provide profiles and overriding built-in menu with text-configurable menu.
2. Directory is just a file. Currently only FAT16/32 are used and filesystem. So it's format of this "file" is the same always. Try to open it as file, parse required info (or just filenames and then get info with safe_stat. it's interesting is safe_stat have same crash problem? :)).
3. Use static binding but in addition have a module which make and raise submenu with modules. This is more safe because it is called only if this menu item is selected.

Unfortunatelly in-module submenus have shortcomings:
a) its items couldn't be binded to main menu, because it exists only when module ran. So it can't be used in user menu, in text-configured overriding menu.
b) I think that modular system is already big difference for user from 1.0 and so it is good opportunity to reorganize and simplify many submenus. May be dynamically. But this will be harder to make when submenus are separated from core.
That's why I didn't move them at start
Logged
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #195 on: 09 / January / 2012, 07:22:44 »
Quote from: tsvstar on 09 / January / 2012, 01:25:12

Just one more note about one of case of changes in export_list. If some exported functions are removed from CHDK in any reason, then correspondent record in export_list should be changed to 0. This both keep compatibility and safety (modules which use such symbols just will not be loaded)


Ok, here's my suggestion to fix the module compatibility being dependent on the order / location of entries in the export list array.

Instead of using the array index as the exported symbol id, generate a 32 bit hash value for the symbol from the name. The hash value is stored in the flat file table instead of the index and the module loader has an array that contains both the hash value and symbol addresses. The loader can then lookup the address from the hash value.

The advantage is that the hash value will never change unless the function name changes. So we can add, remove and re-order the exported list as much as we like.

Disadvantages are that it requires more space for the lookup array and is slower to find a symbol address from the hash value. It is also dependent on finding a hash function that generates a unique value for all the CHDK exported symbols.

The djb2 function at http://www.cse.yorku.ca/~oz/hash.html satisfies the uniqueness criteria (on all current symbols).

I have implemented a test version of this and, using a binary search to find the symbol address, there is no noticeable speed penalty to loading modules.

Thoughts?

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)
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #196 on: 09 / January / 2012, 15:21:38 »
Yes that make more flexible. But this surely take space and  make load algorithm even more complex (especially with binary search).
Moreover last module structure allow to use full names. I just not use because cons bigger than benefits in my opinion. That's why I do not just port elf loader but use max simplified format and algorithm.

I see no problem with limitation to add to the end only and make zero in the middle if remove(I don't think that this will be happen often).
Logical grouping to make good foundation - ok. But why try keep it if elf2flt say what is missed if any?

UPD: Regarding to hash - hash function for strings already exists in current workspace. It is named lang_strhash31. No need to implement one more. I didn't check it for CHDK symbols (I use it for .lng entries only and check them) but suspect that no collision will be.
« Last Edit: 09 / January / 2012, 15:29:14 by tsvstar »
Logged
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #197 on: 09 / January / 2012, 16:43:49 »
Quote from: tsvstar on 09 / January / 2012, 15:21:38
Yes that make more flexible. But this surely take space and  make load algorithm even more complex (especially with binary search).
Moreover last module structure allow to use full names. I just not use because cons bigger than benefits in my opinion. That's why I do not just port elf loader but use max simplified format and algorithm.

There are currently 193 exported symbols, the hash solution adds one int value for each symbol so less than 800 bytes extra for the lookup table. The binary search function is only around 40 bytes so the space overhead is not large. The two modules that were getting values directly from the CHDK_EXPORT_LIST array are changed to call the binary search function to find the symbol address.

With < 200 symbols the binary search is not noticeable in normal use - if you were loading a module hundreds of times a second it would be a problem; but in that case there are better ways to deal with the situation (like not unloading the module).

Storing the symbols names as strings and doing string lookup would require a lot more space and would be a lot slower.

Quote
I see no problem with limitation to add to the end only and make zero in the middle if remove(I don't think that this will be happen often).
Logical grouping to make good foundation - ok. But why try keep it if elf2flt say what is missed if any?

The hash solution is, in my view, a good compromise - it eliminates the hardwired dependency between the symbol name and it's position in the array without adding too much overhead.

The symbol list can now be stored in a text file that is used as input to makeexport.c - we can do whatever we want in regards to ordering the contents and also can add comments to the file (with a small update to the parser in makeexport).

If a symbol is removed the binary search will fail and return 0 as the address so no need to keep placeholders in the array.

It also reduces the burden on developers having to remember (and use) the rules when adding new modules & symbols.

Quote
UPD: Regarding to hash - hash function for strings already exists in current workspace. It is named lang_strhash31. No need to implement one more. I didn't check it for CHDK symbols (I use it for .lng entries only and check them) but suspect that no collision will be.

The hashing is done in makeexport and elf2flt at compile time, not in CHDK at runtime.

I have a bit of cleanup to do in the code, then I will post this version to the reyalp-flt branch so you can take a closer look - when you have time :)

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)
*

Offline outslider

  • *****
  • 762
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #198 on: 09 / January / 2012, 18:32:48 »
Forgive me my ignorance, but having not enough programing skills I don't understand some things with modules. I guess it's better to ask here that start new thread.

I tried to define new int function in gui_draw.c and then use it in module. At the end I tried the simplest possible way to do this, so in:

- gui_draw.c I have:

Code: [Select]
int give_me_value(int a) {
    return a;
}

- appropriate in gui_draw.h:
Code: [Select]
extern int gieve_me_value(int a)


- in any module I tried:
Code: [Select]
int value;
value=give_me_value(4);

And I have this compiler message:

Code: [Select]
In file tbox.elf:
elf2flt unknown symbol: 'give_me_value'
C:\chdk-comp\gcc451\bin\gmake.exe[2]: *** [tbox.flt] Error 5
C:\chdk-comp\gcc451\bin\gmake.exe[1]: *** [all-recursive] Error 1
gmake: *** [all-recursive] Error 1

I can use this function anywhere outside modules. How can I define new function in core and use it in module? I can use draw_xxx functions from core and they're defined in gui_draw.c|h so I thought I can do the same. Should I add new function in a kind of array avialble for modules?
Logged
if (2*b || !2*b) {
    cout<<question
}

Compile error: poor Yorick
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #199 on: 09 / January / 2012, 19:09:31 »
Quote from: outslider on 09 / January / 2012, 18:32:48
Forgive me my ignorance, but having not enough programing skills I don't understand some things with modules. I guess it's better to ask here that start new thread.

I tried to define new int function in gui_draw.c and then use it in module. At the end I tried the simplest possible way to do this, so in:

- gui_draw.c I have:

Code: [Select]
int give_me_value(int a) {
    return a;
}

- appropriate in gui_draw.h:
Code: [Select]
extern int gieve_me_value(int a)


- in any module I tried:
Code: [Select]
int value;
value=give_me_value(4);

And I have this compiler message:

Code: [Select]
In file tbox.elf:
elf2flt unknown symbol: 'give_me_value'
C:\chdk-comp\gcc451\bin\gmake.exe[2]: *** [tbox.flt] Error 5
C:\chdk-comp\gcc451\bin\gmake.exe[1]: *** [all-recursive] Error 1
gmake: *** [all-recursive] Error 1

I can use this function anywhere outside modules. How can I define new function in core and use it in module? I can use draw_xxx functions from core and they're defined in gui_draw.c|h so I thought I can do the same. Should I add new function in a kind of array avialble for modules?

Add it to the array in module_exportlist.c

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)
Pages: 1 ... 15 16 17 18 19 [20] 21 22 23 24 25 ... 32   Go Up
« previous next »
 

Related Topics