search

opendreambox / enigma2-plugins

Python plug-ins for enigma2 (DreamOS)
https://dreambox.de/
Other
37 stars 49 forks source link

[WebInterface] /grab command is broken #23

Open shaxxx opened 4 years ago

shaxxx commented 4 years ago

DISCLAIMER: My dreambox is old and cannot run new webinterface, so there is no way to test this. I'm reporting this based on a code review and user experiences.

In a recent PR https://github.com/opendreambox/enigma2-plugins/commit/a34f4b279a5380d7ded9512e70b9bd57ecce5bae /grab command was renamed with /screenshot (and some parameters changed). My python is weak but as far I can see you did keep loadCompat overload available but this is never called in current implementation (default handler calls 'load'), and /grab controller is no longer working. Please, do correct me if I'm wrong. Like I said, I'm reporting because users of my mobile app cannot use Screenshot command anymore on new Dreambox 900 UHD. You can see command here

https://github.com/shaxxx/EnigmaWeb.Dart/blob/9e7590c06bf72ea06015db63540ff763693ea882/lib/src/commands/screenshot_command.dart#L31

Same thing is used in webbouqueteditor plugin in this repository. https://github.com/opendreambox/enigma2-plugins/blob/fc6144cbefcaf2c74f6ab8bf929b54f1e21f569d/webbouqueteditor/src/web-data/tools.js#L1066

Now, can you please make /grab command available again?

shaxxx commented 4 years ago

Ok, after further investigation problem exists only with OE 2.6 image (Dreambox One).

Grab command gets response, but only OSD us visible (if there is any). Video is not visible. Screenshot in web application works as expected. Again, this is something reported from users. Not something I can check.

drecomx commented 4 years ago

DreamOS for One has version 4.4. You can get it with ip:port/web/about

shaxxx commented 4 years ago

Not sure I follow you. You're suggesting to use /web/about to recognize image version and somehow workaround this (like using /screenshot command instead)? Why? Grab command is not removed, it's broken. Again, cannot confirm it myself. But it's easy to test.

drecomx commented 4 years ago

DreamOS will not use grab anymore in the future. So, I showed you a way to identify DreamOS' version and call the correct url. That's a simple if.

shaxxx commented 4 years ago

Thanks @dre. Not really sure why you decided to make breaking change. I realize grab was never part of the enigma2 API to begin with, but plenty of clients use it. Why not just use /grab as another alias for /screenshot? Translating parameters from one to another should be straightforward.

drecomx commented 4 years ago

I didn't decide anything. But instead of discussing it's often easier to accept the situation and implement workarounds.

shaxxx commented 4 years ago

:) Makes sense. Would it be too much if I asked to copy/paste output of /web/about from Dream OS here? You can leave out mac address.

shaxxx commented 4 years ago

After checking enigma2 and webinterface sources I found that neither /web/about or /web/deviceinfo contain any info about OE version. This information is available only in OpenWebif. So I can't create workaround even if I wanted to. And after seeing multiple commits regarding screenshot in enigma2 master lately I still believe this is bug, not intended behaviour. And once again this is something I cant personally test.

sreichholf commented 4 years ago

We cloud make /grab an alias to /screenshot to work around this for backwards compat. Thx for the input!

shaxxx commented 4 years ago

If I may, I would propose additional feature request. Ignore unsupported parameters. For example, if I call /grab?foo=bar, this should still work, like there's no query parameters. Currently this returns black picture. Simply go trough parameters as you're going now, but when it's not recognized just skip it and go with next one.

This is actually another issue, but I didn't wan't to push my luck.