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:
OctoPrint/src/octoprint/timelapse.py
Lines 768 to 771 in fed2dd7
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.