microsoft/xlang

The parameter is incorrect in find_all_async

debakarr opened this issue ยท 8 comments

Hi,

I want to use WinRT to get a list of all I2C devices in the system. I have the following code:

import asyncio

from winrt.windows.devices.enumeration import DeviceInformation
from winrt.windows.devices.i2c import I2cDevice


async def get_all_devices():
    selector = I2cDevice.get_device_selector()
    devices = await DeviceInformation.find_all_async(selector)
    return devices


if __name__ == "__main__":
    loop = asyncio.new_event_loop()
    asyncio.set_event_loop(loop)
    devices = loop.run_until_complete(get_all_devices())
    for device in devices:
        print(device)

It is failing with this error:

line 10, in get_all_devices
    devices = await DeviceInformation.find_all_async(selector)
RuntimeError: The parameter is incorrect.

What am I doing wrong here?

dlech commented

I have a fork of this project where I have fixed lots of bugs, including serious ones like memory leaks and use after free. It also include type hints which would probably help solve your question as well. You can find current latest CI builds at https://github.com/dlech/xlang/actions/runs/1642108211.

Thanks @dlech I tried your build. Now the error is a bit more descriptive:

line 10, in get_all_devices
    devices = await DeviceInformation.find_all_async(selector)
TypeError: an integer is required (got type str)

Looking at the cpp file for Windows.Devices.Enumeration the logic looks something like this:

static PyObject* DeviceInformation_FindAllAsync(PyObject* /*unused*/, PyObject* args) noexcept
    {
        Py_ssize_t arg_count = PyTuple_Size(args);

        if (arg_count == 3)
        {
            try
            {
                auto param0 = py::convert_to<winrt::hstring>(args, 0);
                auto param1 = py::convert_to<winrt::Windows::Foundation::Collections::IIterable<winrt::hstring>>(args, 1);
                auto param2 = py::convert_to<winrt::Windows::Devices::Enumeration::DeviceInformationKind>(args, 2);

                return py::convert(winrt::Windows::Devices::Enumeration::DeviceInformation::FindAllAsync(param0, param1, param2));
            }
            catch (...)
            {
                py::to_PyErr();
                return nullptr;
            }
        }
        else if (arg_count == 0)
        {
            try
            {
                return py::convert(winrt::Windows::Devices::Enumeration::DeviceInformation::FindAllAsync());
            }
            catch (...)
            {
                py::to_PyErr();
                return nullptr;
            }
        }
        else if (arg_count == 1)
        {
            try
            {
                auto param0 = py::convert_to<winrt::Windows::Devices::Enumeration::DeviceClass>(args, 0);

                return py::convert(winrt::Windows::Devices::Enumeration::DeviceInformation::FindAllAsync(param0));
            }
            catch (...)
            {
                py::to_PyErr();
                return nullptr;
            }
        }
        else if (arg_count == 1)
        {
            try
            {
                auto param0 = py::convert_to<winrt::hstring>(args, 0);

                return py::convert(winrt::Windows::Devices::Enumeration::DeviceInformation::FindAllAsync(param0));
            }
            catch (...)
            {
                py::to_PyErr();
                return nullptr;
            }
        }
        else if (arg_count == 2)
        {
            try
            {
                auto param0 = py::convert_to<winrt::hstring>(args, 0);
                auto param1 = py::convert_to<winrt::Windows::Foundation::Collections::IIterable<winrt::hstring>>(args, 1);

                return py::convert(winrt::Windows::Devices::Enumeration::DeviceInformation::FindAllAsync(param0, param1));
            }
            catch (...)
            {
                py::to_PyErr();
                return nullptr;
            }
        }
        else
        {
            py::set_invalid_arg_count_error(arg_count);
            return nullptr;
        }
    }

Is it only entering the first else if (arg_count == 1) block and not the second one?

dlech commented

Yes, that looks like a problem. It looks like the Python bindings only handle overloads based on the number of arguments, but in this case there are two overloads that take one argument. There is a related discussion at #735 (comment) that describes this in more detail. So I think the solution here is to modify the generator to catch cases like this and create a separate method for one of the overloads.

I am not an expert when it comes to c/cpp but looks like we need to do something in write_method_overloads function in https://github.com/microsoft/xlang/blob/master/src/tool/python/code_writers.h

dlech commented

The discussion I linked to actually suggested handling it before that. Instead of trying to inspect the types in a single method call, the method should be split into 2 methods.

dlech commented

I just found this design note: https://devblogs.microsoft.com/oldnewthing/20210528-00/?p=105259

So I think that may be the simpler solution.

dlech commented

I pushed a change to implement the recommended handling using the DefaultOverloadAttribute.

So in this specific case, the DeviceClass arg is the default overload for the 1 arg overload. So to use the string argument for the first argument, you need to use the 2 argument overload. The second argument is an iterator, so you can just supply an empty list.

await DeviceInformation.find_all_async(selector, [])

This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.