enthought / comtypes

A pure Python, lightweight COM client and server framework, based on the ctypes Python FFI package.
Other
291 stars 96 forks source link

array of BSTR is not supported #347

Open matthiasfinger opened 2 years ago

matthiasfinger commented 2 years ago

Hi, I really appreciate the great work to develop comtypes. Many thanks! I miss one feature which is the transfer of BSTR arrays. I suggest to change the following code lines in automation.py:

elif isinstance(value, (list, tuple)):
            isstr = [isinstance(x,str) for x in value]
            if not False in isstr:
                bstrlist = []
                for item in value:
                    bstrlist.append(cast(SysAllocStringLen(item, len(item)),BSTR))
                obj = midlSAFEARRAY(BSTR).create(bstrlist)
            else:
                obj = _midlSAFEARRAY(VARIANT).create(value)
            memmove(byref(self._), byref(obj), sizeof(obj))
            self.vt = VT_ARRAY | obj._vartype_
junkmd commented 2 years ago

I think this may be related to #80.

Your proposal should be more based on the new code than #80.

What is the COM library stuff that behaves to return an VARIANT that has a VT_ARRAY | VT_BSTR typecode?

I am concerned about how to test to guarantee that a VARIANT with typecode VT_ARRAY | VT_BSTR will be returned.

matthiasfinger commented 2 years ago

In my example I send a BSTR array to a library. Unfortunately it is not freely available. Maybe someone else has an example? One solution would be a Python COM server as test. For me the code works fine.

junkmd commented 2 years ago

@matthiasfinger

It sounds good to have a COM server library for testing.

It seems that the existing COM server testings were also ran that way.

However, these tests, which require running as an administrator or registering in the registry, have been skipped.

At least, I think that it is necessary that the tests run in AppVeyor CI to ensure behavior.

I would like to hear from anyone familiar with AppVeyor and its admin privileges and the registry.

Or do you have any good way to testing?

vasily-v-ryabov commented 2 years ago

AppVeyor build agent should run as Administrator with disabled UAC. Just need to register COM servers in appveyor.yml before running the tests. If something is failed, I can take a look.

matthiasfinger commented 2 years ago

Maybe it would be necessary to implement a way to define the output data type in the future. Just declaring it over the used python type could be insufficient in some cases. For instance by creating and passing a custom safearray. At the moment this possibility is quite hidden and would be needed more at the frontend.

junkmd commented 2 years ago

@vasily-v-ryabov

I know very little about the registry and AppVeyor, so I apologize if I am totally off base.

I think it might be able to register any typelib in the registry this way, is this correct?

https://github.com/enthought/comtypes/blob/7f7c283531ab388f9c68ed7828f5ab99e55fd45a/appveyor.yml#L27-L31

test_script:
   - cd comtypes\test
   - regsvr32 TestComServer.tlb
   - cd ..\..
   - C:\%py%\python.exe setup.py install
   - C:\%py%\Scripts\pip.exe uninstall comtypes -y
   - C:\%py%\python.exe test_pip_install.py
   - C:\%py%\python.exe -m unittest discover -v -s ./comtypes/test -t comtypes\test

I cannot be assure that the may not be compatible with the version of Windows that you're running error dialog will not appear.

vasily-v-ryabov commented 2 years ago

regsvr32 supports only .dll files. You need regasm tool: https://learn.microsoft.com/en-us/dotnet/framework/tools/regasm-exe-assembly-registration-tool?redirectedfrom=MSDN You should be careful with 32-bit or 64-bit version of the tool and Python.

Also I'd suggest to use setUpClass and tearDownClass methods to register and unregister type libraries only for relevant test suites. For example, subprocess.check_output("some command", shell=True) can be useful.

junkmd commented 1 year ago

Is it corrrect?

# comtypes\test\test_comserver.py
REG_PATH = os.path.join(os.path.dirname(__file__), "TestComServer.tlb")

class TestInproc(unittest.TestCase):
    @classmethod
    def setUpClass(cls):
        subprocess.check_output("regsam \"%s\"" % REG_PATH, shell=True)

    @classmethod
    def tearDownClass(cls):
        subprocess.check_output("regsam \"%s\" /unregister" % REG_PATH, shell=True)

I have looked for some examples of using regasm, but they all use .dll files like the ones in MS documentation's Examples.

regasm myTest.dll

junkmd commented 1 year ago

To modify code as @matthiasfinger 's suggestion, test_excel.Test_LateBind will be failed.

...\comtypes> python -m unittest comtypes.test.test_excel.Test_LateBind.test -vv
test (comtypes.test.test_excel.Test_LateBind.test) ... FAIL

======================================================================
FAIL: test (comtypes.test.test_excel.Test_LateBind.test)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "...\comtypes\comtypes\test\test_excel.py", line 62, in test
    self.assertEqual(xl.Range["A1:C3"].Value(),
AssertionError: Tuples differ: ((10.0, 20.0, 31.4), ('x', 'y', 'z'), ('3', '2', '1')) != ((10.0, 20.0, 31.4), ('x', 'y', 'z'), (3.0, 2.0, 1.0))

First differing element 2:
('3', '2', '1')
(3.0, 2.0, 1.0)

- ((10.0, 20.0, 31.4), ('x', 'y', 'z'), ('3', '2', '1'))
?                                        - ^  - ^  - ^

+ ((10.0, 20.0, 31.4), ('x', 'y', 'z'), (3.0, 2.0, 1.0))
?                                         ^^   ^^   ^^

----------------------------------------------------------------------
Ran 1 test in 3.529s

FAILED (failures=1)

Since the property is set to an array of strings, I don't see a problem with a string being returned when a get is done. Nevertheless, this breaks backward compatibility and will be something to keep in our mind when upgrading.

And even after applying this change, test_excel.Test_EarlyBind still fails as reported by @karpierz in #212.

matthiasfinger commented 1 year ago

Would be possible to check whether everything is convertible to a number. But the question for me is: what would be the correct and expected behaviour? Due to the fact that a single value is sent as BSTR, I would expect the same for a tuple, isnt't it?

junkmd commented 1 year ago

@matthiasfinger

It seems to be still necessary to prepare its own dll or tlb for testing to check the expected behavior for Variant arrays.

I think that Excel cell values may be inappropriate to use for testing Variant arrays because they rely on number format. test.test_excel is necessary, but we need to consider what asserting.