on_off is not a descriptive name. There are lots of things on a camera you can turn on and off.
The concept isn't consistent with the existing API. This isn't necessarily a reason to reject it (some of the existing patterns suck, and there are already plenty inconsistencies) but where possible having consistent interfaces is preferable.
String parameters limit the possibility of making the interface compatible between ubasic and lua. You can use literal strings in ubasic, but you can't put them in variables or manipulate them. I'm not necessarily against leaving features out of ubasic, but all else being equal it's preferable to support both. Lua centric interfaces makes sense for things that can't reasonably be implemented in ubasic, or where using Lua's more advanced language features make things significantly simpler or easier to understand.
From a code efficiency point of view, there is little difference in cost between a C function for each thing (set_foo(value)) vs a function that takes named parameters (set("foo",value)). Clarity and maintainability matter much more.
Thanks for the thoughts. I just got the new function working, but I need to clean up the code and do some more testing before posting a patch.
"on_off" may not be the best name, but it describes exactly what the function can do. It can turn anything in the camera on or off, not just the CHDK OSD elements. My current version also turns the backlight off and on. It's very easy to extend the function to turn off/on anything.
The concept is partly consistent with the press, release, and click functions. They all take a string parameter that represents the key name. Using a fixed string in uBasic isn't a problem. I was only manipulating the strings for testing all the options in Lua.
Because the function uses strings instead of "magic" numbers, it should be easier to understand the script. As I said, the function can be expanded to turn anything on and off in the future. It's simple enough that it should be easy to maintain. It's much easier to understand the on_off C code than to figure out the press/release/click C code.
Instead of using +, -, and ~, I'm now using ON, OFF, and TOGGLE in caps. ON, OFF, or TOGGLE can appear anywhere in the string, and unrecognized words are ignored. For example, you could write:
on_off("turn console ON")
or
on_off("TOGGLE backlight")
Similarly, you can enter the item you're changing anywhere in the string. You can change "all", "osd", "title", "console", and "backlight" with the current implementation. I'm still working on "display".
Other possible names for the function might be:
"switch_state"
"switch_status"
"status"
I like the look of:
switch_state("console OFF")
switch_state("TOGGLE backlight")
s=switch_state("display")
The word "switch" can be used as a verb meaning "change", as in the first two examples, or as a noun, as in "get me that switch", like in the 3rd example.
Here's the lua part of the modifications:
//******* on_off function start
static const char *swname[]={"all","osd","title","console","backlight","display",NULL};
static int luaCB_on_off( lua_State* L )
{
const char *stype=luaL_checkstring( L, 1 );
int do_or=0; //1 off, 1 on, 0 toggle, 0 no change
int do_eor=0; //0 off, 1 on, 1 toggle, 0 no change
int i=0;
while(strstr(stype,swname[i])==NULL)
{
i++;
if(swname[i]==NULL)return luaL_error( L, "unknown on_off type" );
}
if(strstr(stype,"ON")!=NULL)do_or=do_eor=1;
else if(strstr(stype,"OFF")!=NULL)do_or=1;
else if(strstr(stype,"TOGGLE")!=NULL)do_eor=1;
int r=0; //result
switch(i)
{
case 0: //all
r=camera_info.state.all_off=(camera_info.state.all_off|do_or)^do_eor;
break;
case 1: //osd
r=camera_info.state.osd_off=(camera_info.state.osd_off|do_or)^do_eor;
break;
case 2: //title
r=camera_info.state.title_off=(camera_info.state.title_off|do_or)^do_eor;
console_redraw(); //move console to or from bottom line
break;
case 3: //console
r=camera_info.state.console_off=(camera_info.state.console_off|do_or)^do_eor;
break;
case 4: //backlight
r=camera_info.state.backlight_off=(camera_info.state.backlight_off|do_or)^do_eor;
if (r) TurnOffBackLight();
else TurnOnBackLight();
break;
case 5: //display
r=camera_info.state.display_off=(camera_info.state.display_off|do_or)^do_eor;
//add display function here
break;
}
if((i<4) && r)gui_set_need_restore(); //clear screen when turning something off
lua_pushnumber(L,r); //change to boolean?
return 1;
}
//******* on_off function end
Here's the rough Lua test script. Everything worked so far.
--[[
@title On_Off 1 Lua
@param d Dummy
@default d 1
--]]
set_console_layout(10,0,45,5)
set_console_autoredraw(2) -- 2 means no console timeout
print("<SET> backlight | <DISP> error")
print("<LEFT> osd | <RIGHT> console")
print("<UP> title line | <DOWN> all")
print("<MENU> exit")
stype={"osd","console","title_line","all","backlight"}
repeat
sp=string.format("a=%s t=%s c=%s o=%s",tostring(on_off("all")),tostring(on_off("title_line")),tostring(on_off("console")),tostring(on_off("osd")))
repeat
draw_string(0,0,sp,264,256)
wait_click(250)
until not is_key("no_key")
print(sp)
if(is_key("display") or is_key("video"))then force_error() end -- call nil function
if(is_key("set"))then on_off("TOGGLE backlight") end
if(is_key("left"))then on_off("TOGGLE osd") end
if(is_key("right"))then
on_off("TOGGLE console")
end
if(is_key("up"))then on_off("TOGGLE title") end
if(is_key("down"))then on_off("TOGGLE all") end
until is_key("menu")
draw_clear()
--flash on off individually
for i=1,5 do
on_off("OFF "..stype[i])
sleep(1000)
on_off("ON "..stype[i])
sleep(1000)
end
