microsoft / xlang

MIT License
877 stars 103 forks source link

The parameter is incorrect in find_all_async #771

Closed debakarr closed 1 year ago

debakarr commented 2 years ago

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 2 years ago

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.

debakarr commented 2 years ago

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 2 years ago

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 https://github.com/microsoft/xlang/issues/735#issuecomment-823585855 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.

debakarr commented 2 years ago

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 2 years ago

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 2 years ago

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 2 years ago

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, [])
github-actions[bot] commented 1 year ago

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.