OctoPrint/OctoPrint

Fix `WebcamProviderPlugin.take_webcam_snapshot` usage in timelapse

Opened this issue · 0 comments

Problem

The timelapse code currently calls that with the full webcam object:

self._logger.debug(
f"Going to capture {filename} from {self._webcam.config.name} provided by {self._webcam.providerIdentifier}"
)
snapshot = self._webcam.providerPlugin.take_webcam_snapshot(self._webcam)

It should however be using the webcam name only.

See also this discussion on Discord:

bchanudet — 05/09/2024 11:39 PM
Hey there, I don't know if that's the right channel but I was wondering about the real prototype of the take_webcam_snapshot() of the WebcamProviderPlugin class. The docs say the first parameter webcamName should be a string, but in timelapse.py it sends the whole ProvidedWebcam instance instead.

I haven't found any other place to confirm which it is really:

  • the classic Webcam plugin doesn't use the parameter
  • in tornado.py, it seems that it sends the webcam name as the first parameter, so a string?

bchanudet — 05/09/2024 11:46 PM
I'm asking because in OctoRant I send a string, but MultiCam is expecting a ProvidedWebcam instance, thus our two plugins cannot work together for now. But I don't know if I should fix it, or if MultiCam should fix it. I haven't yet found an other plugin that use the WebcamProviderPlugin Mixin to collect more evidence towards one or the other prototype.

foosel — Today at 11:14 AM
string is correct, the implementation in timelapse.py is wrong, that shoudl have been webcam.name basically. The idea behind that IIRC was that a webcam provider plugin may provide multiple webcams, so this parameter allows to tell it which webcam it should take the snapshot from. [...]

Solution

Fix timelapse implementation. But since some third party implementation seem to have used that as a "do it like this" approach, also add a big heads-up about this for plugin authors.