picamera2 icon indicating copy to clipboard operation
picamera2 copied to clipboard

RFC: wait/signal_function question

Open meawoppl opened this issue 3 years ago • 1 comments

There are a bunch of methods that take the wait and signal_function arguments. Right now, they have a somewhat confusing set of behaviors:

  • Calls that have both wait=None and signal_function=None defaults DO wait unless you specify one or the other (somewhat unexpected)
  • If you specify wait=True -and- specify a signal_function it will not wait. (very unexpected)

This PR maintains the current behavior, but at least condenses those choices into two places. Thoughts on the best way to specify this all?

My suggestion is changing the default value of wait=None to wait=True and cascading that logic downward. I suspect wait=False is a minority use case which can explicitly override this.

meawoppl avatar Oct 09 '22 19:10 meawoppl

@davidplowman any thoughts on the above?

meawoppl avatar Oct 13 '22 07:10 meawoppl

Hi, thank you for submitting this. I was wondering whether we might break this one up also into a number of separate PRs. Perhaps one for the type-checking changes, another for the re-entrant lock change and then another for the tidying up of the wait/signal_function arguments. Thanks!

BTW, I'm not sure about using a re-entrant lock, I've had it bashed into me so many times in the past that people only use them if they can't be bothered to think about who should or shouldn't be taking the lock... (maybe not true, but there will always be people who keep telling me that!)

davidplowman avatar Oct 17 '22 08:10 davidplowman

My thoughts on the Lock vs. RLock here is mainly looking at code minimization. Just a bit further on, I think most of this could be transformed into input/output queues of futures, which would obviate the whole need.

The processing thread is basically doing that right now, but in a much more complicated way without threadsafe primitives.

meawoppl avatar Oct 17 '22 19:10 meawoppl

Thanks again for everything here. I was wondering if we could take one change at a time. The most obvious thing is that the "waiting" has been moved into dispatch_functions (or _dispatch_functions). Is that a change we could make in its own PR first? Then we could get that merged first before moving on.

Sorry it takes me a while to get round to everything, but as you can probably tell I'm really keen on always doing just a single thing in a commit. Also, for some reason your commit messages are showing up as just "Signed off by ...", is that something you can fix? Thanks!

davidplowman avatar Oct 20 '22 12:10 davidplowman