Unsupported Devices Trigger STDOut on python getVacuumDevices

[mattyarmolich]

Describe The Bug:

see title To Reproduce:

Have a leak sensor, client instanciation will trigger STD out. where its piped into the javascript runtime it short circuits and breaks Expected behavior:

Dont check stderr, scream really loud, reassign to std out for inter package communication. Logs:

[30/11/2022, 21:08:17] [WyzeRoboVac] stderr: Unknown device type detected ({'mac': '79E68BD8', 'first_activation_ts': 1669737831000, 'first_binding_ts': 1669737832000, 'enr': 'j6vs7k0JdufBj3et', 'nickname': 'Sump Pump', 'timezone_name': 'America/Los_Angeles', 'product_model': 'WS3U', 'product_model_logo_url': 'https://wyze-file.s3.us-west-2.amazonaws.com/system-logo/device/small/wyze_icon_device_water_sensor.png', 'product_type': 'LeakSensor', 'hardware_ver': '0.0.0.0', 'firmware_ver': '0.0.0.11', 'user_role': 1, 'binding_user_nickna

not showing rest of info because its irrelevant and PII. Could solve 2 ways. one with reassigning stderr with context manager when client is called (error logs happening in external library) or checking std out for context (much more brittle). Would submit PR but only have work laptops :/)

[mattyarmolich]

fixed super jankly by reassigning sys.stderr to open(os.devnull, "w"). This is super dangerous, and no warranty, but if you know what youre doing this should be helpful for the next guy

[RMCob]

I need more info before I can fix this. Please tell me exactly where (source file, line#) the problem is that you 'changed'. Thanks!

[carTloyal123]

I have also had this problem because the wyze-sdk will output unknown devices to stderr but this is totally fine since the real content comes from stdout anyways. You can basically remove the return statement from exec python calls if they hit stderr messages or check if the stderr contains the keyword unknown:

        exec(`python3 ${this.p2stubs}/getVacuumStatus.py ${this.username} ${this.robovac.config.password} '${this.deviceNickname}'`,
          (error, stdout, stderr) => {
            if (error) {
              this.robovac.log.info(`error: ${error.message}`);
              return;
            }
            if (stderr) {
              this.robovac.log.info(`stderr: ${stderr}`);
              return; <------------------------------------------- REMOVE THIS LINE
            }

I am not saying this is the totally correct way to handle it and it should an issue to raise with @shauntarves of the wyze-sdk since it is not always necessary to print out unknown device info when calling to get all devices.

[RMCob]

The issue as I see it is that the wyze-sdk py package does not recognize your leak sensor as a valid Wyze device, whether it supports it or not. I am going to contact @shauntarves to see it he can update his stuff. Thanks.

[shauntarves]

I'd be happy to add support for ANY device that isn't currently supported. Usually, how that works is I will need you to temporarily share the device with me through the Wyze app so I can play around with calls and understand the data structure. Feel free to email me (or just share the device with this same email address) shaun tarves net

I'm curious to hear more about the logging messages that are causing issues for you. Without digging into this code more, it seems like you're scraping std err/out to get python into JS. Is that correct?

I can certainly make logging changes if you need it so that the unknown device message is more like an info- or debug-level feature. Let me know what you need.

It's so cool to see others using my library. This is really great!

[RMCob]

Shaun, thanks for the quick response. As you can see from the code snippet above, I am checking stderr for any messages and this is what’s causing the issue. If you changed your logging to show unknown device info as a debug-level feature then it would future-proof the logic for future unknown devices yet to come from Wyze until you get to support them officially. Thanks for looking into this.

[RMCob]

@shauntarves Where do we stand with this? If it has been fixed please let me know which version so I can update my dependencies. Thanks!

[carTloyal123]

@RMCob I agree that the issue could be handled better from the Wyze-sdk but is it also a solution to remove the return statement from your stderr check when appropriate? If I ask for the list of devices and the text that comes back is is in stderr you can safely ignore it or print it then continue in the function. It feels like outputting unknown device info to stderr is appropriate for the wyze-sdk to alert a user when they have devices that aren't supported rather than having to enable debug features to discover that the device is not supported. I have adopted these scripts and print but ignore the stderr output which has worked flawlessly while also alerting me to devices that should be requested to @shauntarves for future support. I am curious why this should be debug level when it is useful to a user to know when devices are not supported, thanks for the thoughts

[shauntarves]

Hey all, I think we're trying to tackle this problem from a slightly incorrect angle. My library has generic logging very minimally configured as I feel this is up to the consumer to adapt to their needs. What you are seeing is related to how the logging module is configured by default.

From https://docs.python.org/3/howto/logging.html#advanced-logging-tutorial:

By default, no destination is set for any logging messages. You can specify a destination (such as console or file) by using basicConfig() as in the tutorial examples. If you call the functions debug(), info(), warning(), error() and critical(), they will check to see if no destination is set; and if one is not set, they will set a destination of the console (sys.stderr) and a default format for the displayed message before delegating to the root logger to do the actual message output.

So that means that, in absence of any other logger being configured, the default level is warning and the default destination is sys.stderr. There are loads of Stack Overflow Q/A for how to configure what you're asking for (here and here to start).

This seems like the most reasonable approach to me, since I view this use case (scraping stdout/stderr) as an "implementation detail." Let me know what you think...I'm not opposed to making changes, but that does feel like a last resort in this case.