new branch - CHDK : Elf Edition - Developers wanted - page 14 - General Discussion and Assistance - CHDK Forum  

new branch - CHDK : Elf Edition - Developers wanted

  • 316 Replies
  • 118863 Views
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #130 on: 27 / December / 2011, 13:05:40 »
Advertisements
These little things always turn out to be harder than they look.

Code: [Select]
..
) | chdkptp -c -i
..

Should that be :

Code: [Select]
..
) | chdkptp -c -e
..

In any case,  there might be a problem with bombarding the ptp interface with repeated transfers ?  Playing with these two Windows batch files,  I had to add sleep commands (not available in WinXP) to get anywhere at all - but it still hangs up after a while.

upload_all.bat
Code: [Select]
chdkptp -c -e"u bin\diskboot.bin A/diskboot.bin"
sleep 4
for %%f in (bin/chdk/modules/*.flt) do call process_flt.cmd %%f

process_flt.cmd
Code: [Select]
echo uploading %1
chdkptp -c -e"u bin/chdk/modules/%1 A/CHDK/MODULES/"
sleep 4

Even this causes the camera to get "lost" after four or five file transfers :

Code: [Select]
chdkptp -c -e"u bin/CHDK/MODULES/4wins.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/_dng.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/_rawopsleep 4.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/_rawop12.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/benchm.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/calend.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/curves.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/edgeovr.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/fselect.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/mastmind.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/memview.flt A/CHDK/MODULES/"
sleep 4
5chdkptp -c -e"u bin/CHDK/MODULES/modinsp.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/modmenu.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/mpopup.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/palette.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/reversi.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/snake.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/sokoban.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/tetris.flt A/CHDK/MODULES/"
sleep 4
chdkptp -c -e"u bin/CHDK/MODULES/txtread.flt A/CHDK/MODULES/"
sleep 4

On each successful transfer,  I hear the "USB connect" tone from my PC.  It always hangs after a transfer where that tone does not occur.

I guess this is getting off topic, other than being another thing to work through with ELF format.



Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline reyalp

  • ******
  • 14082
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #131 on: 27 / December / 2011, 14:58:05 »
Are you saying about declaration in conf.c or in conf.h? Because second one is not exposed to user anywhere and only first one required to compatibility saved cfg file and scripts. And this first part (conf.c) is used by modules for same purpose.
I see no comment about binary compatibility of conf.h struct similar to comment for export list. Only conf.c list have such comment.

More detail description:
- If any changes happens in typedef struct conf (at conf.h) then this structure become to be binary different. What happens for module: conf.variable mean *(conf_base_address+variable_offset) and variable_offset is constant. So if any variable (even auxialary and not stored to config) was added not to the end of struct - module will go to wrong adress.
- Config are stored as binary compatible with declaration in conf.c. They stored value-by-value as "id-size-value" structure. Same method I use in module now. I bind to conf variable by its id.
OK, thanks for the clarification, that makes sense. No matter what we do, this is convoluted. This is sort of thing is why I am really not enthusiastic about turning everything into modules right away. The correct approach would be to make the code logically modular first, with nice self contained APIs, and then split it into loadable modules.

@waterwingz:
Continuing here http://chdk.setepontos.com/index.php?topic=6231.msg73142#msg73142
Don't forget what the H stands for.

*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #132 on: 27 / December / 2011, 17:37:46 »
Nice idea. It will logically group some items. And I like it. But I'd like to discuss first, because some nuances exists.

That's why I posted here instead of just updating SVN :)

Quote
One more nuance - converting to struct could affect slightly on size. Because we not use just address of variable but address+offsets. So no result benefit probably will be reached. This item is subject of investigate.

After doing some testing it actually reduces the size of the modules, the core CHDK size remains unchanged.
 
This is using a struct directly (camera_info.variable) rather than a pointer, so it is dependent on finding a fix for the problem with arrays in the struct data not being fixed properly when the module is loaded.

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)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)

*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #133 on: 27 / December / 2011, 19:09:57 »
Try this:
- export conf
- add this line to module_inspector.c near the top of gui_module_draw
        draw_txt_string(0, 1,  conf.reader_file, MAKE_COLOR(SCREEN_COLOR, COLOR_WHITE));
- run CHDK, and open a file in the text reader
- run the module inspector, it should show the file name just opened in the reader

I tested your changes. Everything work ok for simple ints (like conf.reader_color or conf.reader_autoscroll), but broke address for arrays.
Looks like I need slightly improve import information section. Just because this surprise untransparent restriction to developer if do not allow transparent export struct.variable.
Module will change their version and become uncompatible.

It also fails to import properly if the variable reference is not in code.
For example the following from dng.c does not work - it does not adjust the variable address referenced in the data structure.

Code: [Select]
struct dir_entry IFD1[]={
 {0xFE,   T_LONG,       1,  0},
 {0x100,  T_LONG|T_PTR, 1,  (int)&camera_info.raw_rowpix},  // this address not fixed when module loaded
    ...

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)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)


*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #134 on: 27 / December / 2011, 20:54:15 »
Attached is a patch that fixes the module loader to handle references to exported data structures for arrays and when the reference is in the data section.

The problem was that there are three components needed to be able to relocate an external (exported) symbol, the address in the code/data to relocate, the symbol id and an optional offset from the start of the symbol address.

The current system over writes the symbol offset with the symbol id in the .flt file so it does not get adjusted correctly when the module is loaded.

This patch works by merging the symbol id and address to be relocated together into the 32 bit int values stored in the import table. The symbol id is in the top 12 bits and the address in the bottom 20.

This keeps the file size the same and is only a minor change to the loader.

The downside is that it imposes limits on the system:
- max # of symbols that can be exported is 4096
- max size of a single compiled module is 1MB

I think these are reasonable limits; but wanted to get consensus before committing this change to SVN.

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)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)

Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #135 on: 28 / December / 2011, 02:12:58 »
Elegant solution. Proposed limits are reasonable. I found same reason of issue but go into different way.

IMO size of auxilary information has small value:
- It doesn't affect at all to size of loaded module or loading speed.
- We are talking about 200 bytes of additional size  for most complex modules (like dng/fileselector) if use int32+int32 instead for this case. So it doesn't impact to used SD size. And we could implement full-size values to store

At same time:
- Format of FLAT now become incompatible anyway now. So we have to change CFLAT version and we can implement more improvement of format to avoid changes in future.
- For example: it's pretty easy to include symbols name into module. AND I do not plan for now use it in loader [it really affect to complexity-size-speed of core].
 As I say above IMO such size difference of aux info is not a problem. But this will provide some benefits: * maping it could be implemented sometimes in future without format changes; * this provide more debug info which is always good;

PS: If we decide to use your solution, then elf2flt part require to be improved slightly more. We should check that importidx and locoffset are in safe range and raise error if it is not. Just in case.
« Last Edit: 28 / December / 2011, 02:18:03 by tsvstar »

*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #136 on: 28 / December / 2011, 02:33:04 »
Elegant solution. Proposed limits are reasonable. I found same reason of issue but go into different way.

IMO size of auxilary information has small value:
- It doesn't affect at all to size of loaded module or loading speed.
- We are talking about 200 bytes of additional size  for most complex modules (like dng/fileselector) if use int32+int32 instead for this case. So it doesn't impact to used SD size. And we could implement full-size values to store

At same time:
- Format of FLAT now become incompatible anyway now. So we have to change CFLAT version and we can implement more improvement of format to avoid changes in future.
- For example: it's pretty easy to include symbols name into module. AND I do not plan for now use it in loader [it really affect to complexity-size-speed of core].
 As I say above IMO such size difference of aux info is not a problem. But this will provide some benefits: * maping it could be implemented sometimes in future without format changes; * this provide more debug info which is always good;

PS: If we decide to use your solution, then elf2flt part require to be improved slightly more. We should check that importidx and locoffset are in safe range and raise error if it is not. Just in case.

Changing the flat file format is probably safer; but required more work for me to implement and test so I chose the option I posted (also more chance of breaking something).

I'll leave it to you to decide which way to implement this.

I've almost done implementing a 'camera_info' data structure to make the dng module platform independent (once the above fix is in).

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)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)

Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #137 on: 28 / December / 2011, 07:29:05 »
Below is patch which rework flat. Everything is well tested with internal load process debug info and with exploration testing. Works ok. Issue is fixed (but do not check "{0x100,  T_LONG|T_PTR, 1,  (int)&camera_info.raw_rowpix},  // this address not fixed when module loaded")

I ready to commit it. Or you can do that.

- Fixed issue with offset (and so importing of conf.variable become valid). Extended import record struct instead of combine in uint32.
- Unified header member value meaning + removed duplications (which was legacy from BFLAT headers)
- Added symbolname section [Not filled right now but no changes will require in future]
- More information produced by elf2flt/fltdump (if correspondent switches are active)

I think this is good idea to commit rearrangement of export_list (including your _cam_info) right after this changes. Module format become different so we should no worry to make it compatible with previous revision, and so we also could make another changes which broke compatibility
« Last Edit: 28 / December / 2011, 07:35:26 by tsvstar »


Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #138 on: 28 / December / 2011, 13:49:36 »
One more proposition:
- Apply patch1509_fltv6 to trunk but commit result to reyalp-flt branch
- Then apply your _cam_info changes also to reyalp-flt.

Reason: I have idea to make one more improvement which broke modules compatibility. Something similar to proposition of reyalp.

Short description:
To make modules version-compatible for data structures (like _cam_info or binding modules structures) common mechanizm will be useful.

Data structure should contain uint32 variable. This variable is inited with #define MAKE_API_VERSION(major,minor) ((major<<16)+minor).

Modules/Core which import data structure should make check
if ( !API_VERSION_MATCH_REQUIREMENT( api_ver, req_major, req_minor ) )
  error

#define API_VERSION_MATCH_REQUIREMENT( api_ver, req_major, req_minor ) ((api_ver>>16)==req_major && (api_ver&0xffff)>=req_minor)

If logic is changed in the way when back compatibility is not possible then major version changed in api version initializer. If something extended (back-compatible) then minor version changed.

PS - I still don't think that such way is good for conf, because it is oftenly changed structure. By-value binding is safe and agile method.
It also could be used but not required for core chdk export_list (base limitation and check are enough + chdk ver check just for user-friendly info and out-of-export_list logic changes).
But this thing surely will provide good safety level for other data structures: _cam_info, librawop, export_list of modules, etc
« Last Edit: 28 / December / 2011, 14:04:06 by tsvstar »

*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #139 on: 28 / December / 2011, 16:00:20 »
One more proposition:
- Apply patch1509_fltv6 to trunk but commit result to reyalp-flt branch
- Then apply your _cam_info changes also to reyalp-flt.

Reason: I have idea to make one more improvement which broke modules compatibility. Something similar to proposition of reyalp.

Short description:
To make modules version-compatible for data structures (like _cam_info or binding modules structures) common mechanizm will be useful.

Data structure should contain uint32 variable. This variable is inited with #define MAKE_API_VERSION(major,minor) ((major<<16)+minor).

Modules/Core which import data structure should make check
if ( !API_VERSION_MATCH_REQUIREMENT( api_ver, req_major, req_minor ) )
  error

#define API_VERSION_MATCH_REQUIREMENT( api_ver, req_major, req_minor ) ((api_ver>>16)==req_major && (api_ver&0xffff)>=req_minor)

If logic is changed in the way when back compatibility is not possible then major version changed in api version initializer. If something extended (back-compatible) then minor version changed.

PS - I still don't think that such way is good for conf, because it is oftenly changed structure. By-value binding is safe and agile method.
It also could be used but not required for core chdk export_list (base limitation and check are enough + chdk ver check just for user-friendly info and out-of-export_list logic changes).
But this thing surely will provide good safety level for other data structures: _cam_info, librawop, export_list of modules, etc

I've tested your patch and it looks fine. I'll commit this and my other changes to the reyalp-flt branch for review.

The API version seems reasonable and won't add much overhead.

It occurred to me that using a data structure might also be an alternate way to handle imports from the module. For raw_merge.c you have the librawop struct defined in the core code and filled manually from the import list in the module. If you move the struct into the module and have a pointer to it in the core code then you only need to import one item (the struct pointer). You might also include the ModuleInfo stuff at the start of the struct as well - essentially a semi object oriented module approach / API.

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)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)

 

Related Topics