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

philmoz

3332

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #210 on: 11 / January / 2012, 08:03:58 »

Advertisements

I have added a new feature for modules to the reyalp-flt branch.

This now allows modules to have their own config file for module specific settings.

This further separates the module from the core CHDK code and allows the module to be extended with new settings without needing to change the CHDK core.

The module config files are stored in A/CHDK/MODULES/CFG and are loaded when the module is loaded. The settings are saved when the module is unloaded.

Settings have been moved to the zebra, edge overlay, grids, text reader and sokoban modules.

The version for the conf array has been updated to 2.0.

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)

tsvstar

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #211 on: 11 / January / 2012, 09:09:34 »

Number of files on SD grow quickly

In one hand it is really valuable because decrease dependencies between modules and core.

In other hand this create difficulties similar to modular submenu (and futher modular language strings) - no common id exists. And so module-specific settings are unavailable from core and scripts.
Possible real example: One of planed module is script-based configurable menu main purpose of which is quick modes switching (selected mode call script which tune config for scene. Similar to camera programs, but much more controlable). Now many config options become unreachable from scripts.

Logged

philmoz

3332

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #212 on: 11 / January / 2012, 14:00:59 »

Quote from: tsvstar on 11 / January / 2012, 09:09:34

In other hand this create difficulties similar to modular submenu (and futher modular language strings) - no common id exists. And so module-specific settings are unavailable from core and scripts.
Possible real example: One of planed module is script-based configurable menu main purpose of which is quick modes switching (selected mode call script which tune config for scene. Similar to camera programs, but much more controlable). Now many config options become unreachable from scripts.

I can understand wanting to save/restore shooting parameters and perhaps CHDK overrides.

But I'm not sure I can see a reason why you would ever want to script any of the internal settings for any of the modules I've updated (sokoban level, zebra colors, etc).

It could still be done with a little bit more work - you need to supply the module name along with the setting id when you want to get / set one of the values, and add a method to expose the modules internal ConfInfo array.

Phil.

Logged

Dave1116

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #213 on: 11 / January / 2012, 14:46:20 »

In this makefile there is a commnet about order as follows:

CFLAGS+=$(CTHUMB) -mlong-calls
# warning: library order matters!
LDLIBS= -lgcc
LDOPTS=-nostdlib -Wl,--allow-shlib-undefined -Wl,-T,$(topdir)tools/link-boot.ld
LDOPTS+=-Wl,-N,-Ttext,0x0016ee30 -r -Wl,-d

OBJS=../gui_calendar.o ../gui_bench.o ../gui_4wins.o ../gui_mastermind.o ../gui_reversi.o \
../gui_sokoban.o ../gui_read.o ../gui_debug.o ../gui_tetris.o ../gui_snake.o ../gui_games.o ../dng.o

My question, does this refer to just the "OBJS=../" or to the elf's that follow further down the file and where do i get the order?

Dave1116

Logged

Canon a1100is 100c, sd1200is 100c

Wikipedia - Hack (computer science), an inelegant but effective solution to a computing problem.

tsvstar

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #214 on: 11 / January / 2012, 15:35:04 »

Unfortunatelly my big post is lost because forum request to login again. So will be short.

I had time to more precise review of 1534. I didn't check new modules (grid,zebra,mdetect) but have few significant notes for other code:

1. What is purpose of addition to module_load.c below?
278 if ( callback )
279 callback( (void**) modules[idx]->_module_exportlist );

This if case mean "module load request but module is already loaded". Only one additional things that we need is sanity unload request (module is unloaded ~40 msec after this is requested) so it is theoretically possible situation when module request for its unload, but something in other thread request its loading right after.
But no re-binding required because module is already binded.

2. Most libraries could be safely unloaded automatically (they just will be loaded again when reqiured) but some of them can't.
In which conditions could happens automatical unloading:
a) When gui mode switched to GUI_MODE_NONE then core unload all modules without MODULE_FLAG_DISABLE_AUTOUNLOAD flag
b) When user leave module submenu then module also is unloaded.

What bad thing happens:
a) DNG library. MODULE_FLAG_DISABLE_AUTOUNLOAD is cleaned so it will be unloaded after user go to menu or just alt mode.
Bad things happened:
- badpixel file is loaded only on CHDK start(config read) or dng bool menu item switched. So it will be lost and no badpixel removal will be applied to following raws.
- perfomance impact (library will be loaded inside shooting process instead of loading on start+ possible loading badpixel in _module_loader to fix first problem also take time and use probably unsafe fopen)

b) EDGEOVR. Shouldn't be unloaded ever. But it will be when enter and leave its submenu
Bad things happened:
- Current edge picture will be lost (release_1_0 never lost this picture. even if turn off edgeovr temporary)
- Memory leak because no free allocated resources in _module_unloader
Simple solution: Move edgebuf into core

3. That is correct that mdetect.flt have MODULE_FLAG_DISABLE_AUTOUNLOAD because it unsafe if it will be unloaded in progress.
I'm not sure why did you set flag MODULE_FLAG_DISABLE_AUTOUNLOAD for zebra, but would like to note just in case that this flag will not prevent unloading when leave its submenu.

Quote from: philmoz on 11 / January / 2012, 14:00:59

But I'm not sure I can see a reason why you would ever want to script any of the internal settings for any of the modules I've updated (sokoban level, zebra colors, etc).

I do agree that this is uncommon case

Logged

tsvstar

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #215 on: 11 / January / 2012, 15:37:10 »

Quote from: Dave1116 on 11 / January / 2012, 14:46:20

In this makefile there is a commnet about order as follows:

CFLAGS+=$(CTHUMB) -mlong-calls
# warning: library order matters!
LDLIBS= -lgcc
LDOPTS=-nostdlib -Wl,--allow-shlib-undefined -Wl,-T,$(topdir)tools/link-boot.ld
LDOPTS+=-Wl,-N,-Ttext,0x0016ee30 -r -Wl,-d

OBJS=../gui_calendar.o ../gui_bench.o ../gui_4wins.o ../gui_mastermind.o ../gui_reversi.o \
../gui_sokoban.o ../gui_read.o ../gui_debug.o ../gui_tetris.o ../gui_snake.o ../gui_games.o ../dng.o

My question, does this refer to just the "OBJS=../" or to the elf's that follow further down the file and where do i get the order?

Dave1116

This makefile was made by me based on the core/Makefile. I just didn't remove this line just in case.
No limitation of order in OBJS or elf or flt exists in this makefile.

« Last Edit: 11 / January / 2012, 15:40:01 by tsvstar »

Logged

msl

1276
A720 IS, SX220 HS 1.01a

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #216 on: 11 / January / 2012, 16:25:55 »

Quote from: philmoz on 11 / January / 2012, 14:00:59

I can understand wanting to save/restore shooting parameters and perhaps CHDK overrides.

But I'm not sure I can see a reason why you would ever want to script any of the internal settings for any of the modules I've updated (sokoban level, zebra colors, etc).

The split of configuration I see critical. Internal settings are not the problems. But we have extra developed commands to read and save of the configuration data. With get/set_config_value we have a easy way for setting different configurations - see this script. E.g. a special script configuration for a pano function with edgeoverlay is no longer possible.

msl

Logged

CHDK-DE: CHDK-DE links

Dave1116

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #217 on: 11 / January / 2012, 16:32:41 »

Thanks, ...

Just compiled branch reyalp-flt 1542:

Extra P O's= ok
Video parms = ok
Raw parms = ok
Edge overlay = no work
Histo = ok
Zebra = no work
OSD = no editor, no grids,
Visual = some work, some not
Scripting = some work, some not, no loading
Miscell. = File browse = no, Module inspector = no,Text file reader = no, Draw Palette = no, Make card boot = yes, Swap part = (no try), Debug look ok, Remote look ok (no try)

Dave1116

IMG_0878.JPG (293.92 kB, 1600x1200 - viewed 74 times.)
Model:Canon PowerShot SD1200 IS Exposure Time:1/40 seconds F Number:F/4.9 ISO Speed:320 Date Taken:2012:01:11 17:05:50 Metering Mode:Pattern Flash Mode:Flash did not fire; compulsory flash mode Focal Length:18.6 mm Color Space:sRGB

Logged

Canon a1100is 100c, sd1200is 100c

Wikipedia - Hack (computer science), an inelegant but effective solution to a computing problem.

philmoz

3332

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #218 on: 11 / January / 2012, 16:46:31 »

Quote from: tsvstar on 11 / January / 2012, 15:35:04

1. What is purpose of addition to module_load.c below?
278 if ( callback )
279 callback( (void**) modules[idx]->_module_exportlist );

This if case mean "module load request but module is already loaded". Only one additional things that we need is sanity unload request (module is unloaded ~40 msec after this is requested) so it is theoretically possible situation when module request for its unload, but something in other thread request its loading right after.
But no re-binding required because module is already binded.

This caters for the scenario where the module is loaded with gui_menu_run_fltmodule to access it's sub-menus. In this case the function in 'modules.c' was not used to load the module so it's bind function was not called. If the module is not unloaded when the menus exit this ensures the bind function is called for the already loaded module.

Quote

2. Most libraries could be safely unloaded automatically (they just will be loaded again when reqiured) but some of them can't.
In which conditions could happens automatical unloading:
a) When gui mode switched to GUI_MODE_NONE then core unload all modules without MODULE_FLAG_DISABLE_AUTOUNLOAD flag
b) When user leave module submenu then module also is unloaded.

What bad thing happens:
a) DNG library. MODULE_FLAG_DISABLE_AUTOUNLOAD is cleaned so it will be unloaded after user go to menu or just alt mode.
Bad things happened:
- badpixel file is loaded only on CHDK start(config read) or dng bool menu item switched. So it will be lost and no badpixel removal will be applied to following raws.
- perfomance impact (library will be loaded inside shooting process instead of loading on start+ possible loading badpixel in _module_loader to fix first problem also take time and use probably unsafe fopen)

b) EDGEOVR. Shouldn't be unloaded ever. But it will be when enter and leave its submenu
Bad things happened:
- Current edge picture will be lost (release_1_0 never lost this picture. even if turn off edgeovr temporary)
- Memory leak because no free allocated resources in _module_unloader
Simple solution: Move edgebuf into core

The menu code only unloads modules that have the module index stored in the menu stack array (see the call to gui_activate_sub_menu). DNG does not have a menu so is not affected by this - there is no change to the DNG load/unload behaviour.

Personally I don't see a problem with losing the current edge buffer if you access the edge menu to change a setting; but I will revisit this to keep the functionality consistent.

Update: Changeset 1562 now saves the edge state to the core CHDK if the edge module is unloaded. The state is restored when the module reloads.

Quote

I'm not sure why did you set flag MODULE_FLAG_DISABLE_AUTOUNLOAD for zebra, but would like to note just in case that this flag will not prevent unloading when leave its submenu.

Unloading the module when exiting the menu ensures the config changes get saved - the module will be re-loaded when next used. If the zebra was turned off then it won't get re-loaded and so this also serves to automatically release the module if it is no longer needed (same applies for edge overlay).

I thought setting the flag was to ensure the module did not get unloaded during normal operation?

Phil.

« Last Edit: 15 / January / 2012, 05:10:26 by philmoz »

Logged

philmoz

3332

Re: new branch - CHDK : Elf Edition - Developers wanted

« Reply #219 on: 11 / January / 2012, 16:51:17 »

Quote from: msl on 11 / January / 2012, 16:25:55

Quote from: philmoz on 11 / January / 2012, 14:00:59
I can understand wanting to save/restore shooting parameters and perhaps CHDK overrides.

But I'm not sure I can see a reason why you would ever want to script any of the internal settings for any of the modules I've updated (sokoban level, zebra colors, etc).

The split of configuration I see critical. Internal settings are not the problems. But we have extra developed commands to read and save of the configuration data. With get/set_config_value we have a easy way for setting different configurations - see this script. E.g. a special script configuration for a pano function with edgeoverlay is no longer possible.

msl

In my opinion the benefits of encapsulating as much as possible related to a module inside the module itself are significant.

However if the general consensus is that this is bad or will cause too many problems I'm happy to revert back.

As I said earlier there are ways to get access to the module internal settings if really needed - although it will probably require revision of the script (need to specify the module name as well as the setting index).

Phil.

Logged