new branch - CHDK : Elf Edition - Developers wanted - page 13 - General Discussion and Assistance - CHDK Forum
« previous next »
Pages: 1 ... 8 9 10 11 12 [13] 14 15 16 17 18 ... 32   Go Down

new branch - CHDK : Elf Edition - Developers wanted

  • 316 Replies
  • 132747 Views
*

Offline reyalp

  • ******
  • 14128
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #120 on: 26 / December / 2011, 15:43:27 »
Advertisements
Quote from: tsvstar on 26 / December / 2011, 05:59:33
This complex solution is required because we can't guarantee that conf structure is binary same on different modules. New conf variables could be added at any place, size of existed members could be changed.
No, this is absolutely not allowed. New / changed conf are only allowed at the end, as the comments in conf.c say.  Sometimes we've added additional values to enums (e.g. conf.foo allowed 0,1,2, in one version, and allows 0,1,2,3 in a later version) but existing items should never be redefined or order changed. This will break things horrible for users when they update.

conf is supposed to be binary compatible with later versions.
« Last Edit: 26 / December / 2011, 15:46:41 by reyalp »
Logged
Don't forget what the H stands for.
*

Offline philmoz

  • *****
  • 3450
    • Photos
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #121 on: 26 / December / 2011, 15:46:37 »
In similar vein as reyalp's suggestion to simplify the interfaces and define API's I'd like to suggest another idea.

At the moment the export list has specific exports for some of the camera.h values like CAM_BLACK_LEVEL, CAM_WHITE_LEVEL, CAM_RAW_ROWS etc. It also has internal variables to store these values so the modules can have something to link to.

Instead it might be cleaner to encapsulate these values into a 'camera_info' structure and just export that.

Code: [Select]
typedef struct {
   int black_level;
   int white_level;
   int raw_rows;
   int raw_rowpix;
      ...
} _cam_info;

_cam_info _camera_info = { CAM_BLACK_LEVEL, CAM_WHITE_LEVEL, CAM_RAW_ROWS, CAM_RAW_ROWPIX, ... };

_cam_info *camera_info = &_camera_info;       // **** Export camera_info ****


This reduces the number of exports while still being extensible.

Module code using the #define values can use the camera_info->xxx variables instead.

The DNG module implementation currently is not portable because of all the camera.h / platform_camera.h defines that are used in the code (CAM_JPEG_WIDTH, CAM_JPEG_HEIGHT, CAM_COLORMATRIX1, CAM_DNG_LENS_INFO, CAM_ACTIVE_AREA_xx, etc).

The above change would support making the DNG code portable without bloating the export list.

This could also be used for the palette colors and probably other things.

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 waterwingz

  • ******
  • 12140
    • Waterwingz
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #122 on: 26 / December / 2011, 19:56:39 »
Quote from: philmoz on 13 / December / 2011, 19:02:25
Quote from: waterwingz on 13 / December / 2011, 18:31:09
One small consequence of all this is that it will no longer be possible to update a camera by simply copying the diskboot.bin file over the USB port via chdkptp.   Rather than copying all the module files over ptp manually,  I guess its back to swapping SD cards if this all becomes standard ?
I use a text file of chdkptp commands to update the camera files - easily extended to include the modules.

chdkptp command line:
Code: [Select]
chdkptp.exe -i -c < update_cam.txt

command file:
Code: [Select]
upload bin\diskboot.bin A/diskboot.bin
upload bin\ps.fi2 A/ps.fi2
reboot
I have batch files to do the above so I don't have to type the command line all the time.
As the number of modules grows, this becomes a challenge to do with a simple batch file as chdkptp does not appear to support wild cards.

I guess the next step is a fancier script that parses the CHDK/MODULES directory and builds a list of script commands to transfer each module one-by-one.

Unless somebody has a simpler way (other than swapping the SD card in & out) ?


Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline reyalp

  • ******
  • 14128
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #123 on: 26 / December / 2011, 20:46:01 »
Quote from: waterwingz on 26 / December / 2011, 19:56:39
As the number of modules grows, this becomes a challenge to do with a simple batch file as chdkptp does not appear to support wild cards.
Unfortunately, the stock lua os lib does not support directory listing. You could do something with io.popen or io.tmpfile and os.execute to list the files externally.
Quote
I guess the next step is a fancier script that parses the CHDK/MODULES directory and builds a list of script commands to transfer each module one-by-one.
Code: [Select]
#!/bin/bash
(
echo u bin/DISKBOOT.BIN
if [ -f bin/PS.FI2 ] ; then
echo u bin/PS.FI2
fi
if [ -f bin/PS.FIR ] ; then
echo u bin/PS.FIR
fi

for f in core/modules/*.flt ; do
echo u $f CHDK/MODULES/
done
) | chdkptp -c -i
Logged
Don't forget what the H stands for.
*

Offline waterwingz

  • ******
  • 12140
    • Waterwingz
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #124 on: 26 / December / 2011, 21:50:26 »
Quote from: reyalp on 26 / December / 2011, 20:46:01
Quote
I guess the next step is a fancier script that parses the CHDK/MODULES directory and builds a list of script commands to transfer each module one-by-one.
Code: [Select]
#!/bin/bash
(
echo u bin/DISKBOOT.BIN
if [ -f bin/PS.FI2 ] ; then
echo u bin/PS.FI2
fi
if [ -f bin/PS.FIR ] ; then
echo u bin/PS.FIR
fi

for f in core/modules/*.flt ; do
echo u $f CHDK/MODULES/
done
) | chdkptp -c -i
Thank you.  So as not to look a gift horse in the mouth,  I'll take some time tomorrow to convert to a Windows version.  The bash shell loaded by CHDK-Shell in its command box chokes on this. I guess I was just being lazy in any case.

Logged
Ported :   A1200    SD940   G10    Powershot N    G16
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #125 on: 27 / December / 2011, 02:21:45 »
reyalp,

Quote from: reyalp on 26 / December / 2011, 15:28:10
Thanks for that, I wondered about that after I did it. And yeah, I should have read the comments :)
Thats why I worry about conf binary compatibility. I like to give no chance to make mistake. :)
Unfortunatelly for modules this mean export by symbol which increase its internal complexity significatelly.

Quote
I would be strongly tempted to export symbols by name. Yes, it takes some RAM, but by going to modules, we've save a lot of RAM. In some cases, we can re-factor the interfaces so not so many exports are needed. Alternately, you could build a symbol map file with each build, then the loader could refer to that (but I don't think there's a need for this yet)
Maybe this will be in next version of module system.
It was implemented in this way because this simplify greatly file format, module loader, save size of core and load speed.

Quote
I'd also suggest we might want to break things up into specific APIs. So a module that only needs stdio can just say it needs stdio, and changes in other areas won't affect it.
It is not clear for me. Now only required to module symbols are affected. If nothing was changed for them, everything will work ok.

Quote
Module inspector would be a good option for debug shortcut used for ram dump, prop/param paging etc.
Yes. That's why I add this specific keyboard handler for my camera. :)

Quote from: reyalp on 26 / December / 2011, 15:43:27
But existing items should never be redefined or order changed. This will break things horrible for users when they update. conf is supposed to be binary compatible with later versions.
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.
« Last Edit: 27 / December / 2011, 05:44:26 by tsvstar »
Logged
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #126 on: 27 / December / 2011, 02:37:15 »
philmoz,

Quote from: philmoz on 26 / December / 2011, 15:29:45
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'll check this. It should work and it worked at early stages of module system (until I isolate conf with binds)

Quote from: philmoz on 26 / December / 2011, 15:46:37
At the moment the export list has specific exports for some of the camera.h values like CAM_BLACK_LEVEL, CAM_WHITE_LEVEL, CAM_RAW_ROWS etc. It also has internal variables to store these values so the modules can have something to link to.
Instead it might be cleaner to encapsulate these values into a 'camera_info' structure and just export that.

This could also be used for the palette colors and probably other things.
Nice idea. It will logically group some items. And I like it. But I'd like to discuss first, because some nuances exists.

Current implementation have two goal:

- Minimize required changes in modules.
As you can see we change nothing in modules, while palettes are completely isolated. Now makeexport detect declarations in first section of module_export.c and make corresponded undefs/declarations in module_export.h to replace defines with new symbol. So while module use same symbols they are no defines anymore but imported variables. This also have benefit of unification with core. We use same symbols as in the core.

- All required safety checks are done by module_system.
I should no worry about _cam_info, _palette version. Are there required symbol exists in this core or not. Every checks will be done by module loader. Module will work ok on any CHDK version which is compatible.
For struct - I will need to set and check version (to avoid call unexisted sym), I should know starting from which version all required symbols are exists (to give possibility run module on previous CHDK which in really are compatible).

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.

Also IMO exporting of defines is quite simple now:
- Declare initialized global variable in "section 2" following to pattern.
- Add this variable into export list ("section 3")
- that's all. Ensure that module_load.c is last included file in the module and change nothing more.

Are 144 saved bytes (for now, and ~300 bytes if most of current camera defines will be exported) in export symbol table is good goal for such changes?
« Last Edit: 27 / December / 2011, 03:11:20 by tsvstar »
Logged
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #127 on: 27 / December / 2011, 03:58:51 »
Resume in few words:

Current implementation:
+ Unified with core usage of symbol. No change required.
+ Automatically binary safe and it same time allow to use any old CHDK which in real is compatible
- Unobvious is define used or this is imported variable
- Unobvious that we export define

Proposed by philmoz changes:
+ Logical grouping of some "API".
+ Obvious and clear that we use imported variable. No conflicts with defines
- Different from core symbols are used (so duplicate "API")
- Slightly more limited in versioning and require additional attention to keep in mind that this struct strong version-dependend.

As for me for _cam_info benefits and cons are equal for both solution. And probably second one is even prefered because more obvious. This symbols are used for complex camera-specific modules and so it is good to make things clear.

But for palette current style should be used to keep unification with core and simplify to all developers module creation. Implementation details (that symbols are not defines) should be hidden (as they are now)
Logged
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #128 on: 27 / December / 2011, 05:36:17 »
Quote from: philmoz on 26 / December / 2011, 15:29:45
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.
« Last Edit: 27 / December / 2011, 05:41:18 by tsvstar »
Logged
*

Offline tsvstar

  • ***
  • 231
Re: new branch - CHDK : Elf Edition - Developers wanted
« Reply #129 on: 27 / December / 2011, 06:56:26 »
Quote from: vnd on 26 / December / 2011, 13:34:30
2. With Makefile.1 almost all modules failed. The attached file contains the results + log.
My fault. It was prepared for different revision. Please try attachment below on gcc4.3.3.
Logged
Pages: 1 ... 8 9 10 11 12 [13] 14 15 16 17 18 ... 32   Go Up
« previous next »
 

Related Topics


SimplePortal © 2008-2014, SimplePortal