search

reticulatedpines / magiclantern_simplified

A Git based version of Magic Lantern, for those unwilling or unable to work using Mercurial. The vast majority of branches have been removed, with those thought to be important brought in individually and merged.
GNU General Public License v2.0
142 stars 47 forks source link

LUA: Add lens.continuous_af #122

Open wow0000 opened 6 months ago

wow0000 commented 6 months ago

Changes

Why

This changes is useful to properly handle errors with the lens.focus() call. It avoids triggering an error and it enable proper error handling.

Testing

The code has been tested on a Canon 100D and does not generate any additional warnings or error. It should not impact current features or existing scripts.

reticulatedpines commented 6 months ago

This isn't an area of code I know well, so I need to ask some questions to determine what this change is supposed to do, and why it might be useful.

What error occurs on lens.focus() call without this change? What improvement in ML behaviour happens after the change?

This existing line suggests that continuous AF is not compatible with lens.focus(): if (is_continuous_af()) return luaL_error(L, "lens.focus() requires %s AF disabled.", is_movie_mode() ? "movie servo" : "continuous");

Is that true? Is it still true now, or does this change alter behaviour? If so, why?

Thanks

reticulatedpines commented 6 months ago

I've spent some time trying to understand this code and something looks wrong here. You're adding a "continuous_af" field to the "lua_lens_fields" struct, but "continuous_af" isn't a property of the lens, it's a camera state. So it definitely shouldn't live in a struct that records information about the lens.

See "autofocusing": https://github.com/reticulatedpines/magiclantern_simplified/blob/04e752b9af67c5b06c2b81cf7b43f97b89a73a3d/modules/lua/lua_lens.c#L70

That's setting something based on camera state, and doesn't add it to the lens struct. Do you really need to add it there?

It would also help if you show me the Lua script you're having problems with. As is, I only have half the information.