static int action_stack_AS_WAIT_SHOOT_DONE()
{
if (shooting_in_progress()) return 0;
action_pop_func();
if(camera_info.state.state_shooting_progress == SHOOTING_PROGRESS_NONE)
{
action_pop_func(); //pop extra delay
action_pop(); //off the stack
action_push_func(action_stack_AS_SHOOT);
script_console_add_line((long)"SHOOT FAILED"); //for debugging
}
return 1;
}
After considering all the ideas presented here, I think this is the best way to fix the shoot() problem. The other proposed solutions add unnecessary code and complexity to the shoot() function.
My basic design philosophy is to figure out what the real world problem is before writing any code. I spend a lot more time thinking of the best way to solve a problem than coding. I throw out 90% of the code I write (sometimes 100%) when I think up a better solution. This is a better solution.
The real world problem is that shoot() fails very rarely on certain cameras under certain conditions. The solution above fixed this problem on the only known camera where we could reproduce the problem reliably. It fixed it with one re-shoot every time, and never hung re-shooting over and over.
The possibility of a re-shoot infinite loop hang is a theoretical coding problem, not a real world problem. The solution above is a big improvement to the current shoot() function in that it corrects the problem in the tests we can do now. It is certainly no worse than the current shoot() function, which just hangs.
If the shoot() function ever enters a continuous re-shoot loop, it will be obvious what happened, unlike the silent hanging of the script that happens now. If this ever happens, then it becomes a real world problem and would be worth adding complexity and coding to correct.
In addition to the complexity in the C code required to implement the other proposed solutions, they add complexity to the script code to account for the possibility of an error return. Again, until shoot() actually hangs in a re-shoot loop, adding this complexity is not worthwhile.
As for the issue of coding clarity, the solution is to change the name of the function from AS_WAIT_SAVE to AS_WAIT_SHOOT_DONE. This describes what the function actually does, and makes it clear that it's tightly coupled to AS_SHOOT. The code is much simpler and easier to follow this way than the other proposed solutions.
I agree that the shoot() change should be separate from the sleep fix. Let's put this simple shoot() fix in the trunk right now on its own and see what happens. If you agree, I'll test it again with this precise code and submit a diff for inclusion in the trunk.
Another thing... This statement, the final delay, in AS_SHOOT isn't necessary. You don't need to wait for the file to be saved, which this delay doesn't do anyway. The next shoot() call waits for shooting_in_progress(), which should be sufficient. Am I wrong?
// XXX FIXME find out how to wait to jpeg save finished
action_push_delay(conf.script_shoot_delay*100);