Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement record pointers as [in, out] method parameters of a Dispatch Interface #2310

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Implement record pointers as [in, out] method parameters of a Dispatch Interface #2310

wants to merge 3 commits into from

Conversation

geppi
Copy link

@geppi geppi commented Jul 22, 2024 •
edited
Loading

to get server side modifications of a record marshaled back to the client.

COM interfaces with an [in, out] method parameter of type pointer to record can get
server side changes to record fields marshaled back to the client.

The current implementation of win32com does not support this and drops
an indirection level on pointer to records, i.e. the VT_BYREF attribute from a VT_RECORD.

This pull request changes the handling of record pointers and enables server side modifications of record fields to be visible on the client side. The standard type library marshaler does marshal those server side modifications back to the COM client when the VARIANT type of a parameter in the DISPPARAMS structure is (VT_BYREF | VT_RECORD).

This pull request resolves #2304.

@geppi
Implement record pointers as method parameters of a Dispatch Interface
22b28ab 
to get server side modifications of a record marshaled back to the client.
@geppi geppi changed the title Implement record pointers as method parameters of a Dispatch Interface Implement record pointers as [in, out] method parameters of a Dispatch Interface Jul 25, 2024
@geppi
Copy link
Author

geppi commented Jul 26, 2024

I was hoping to receive some feedback about what would be required to merge this PR and have in the meantime taken a look into the testing facility of this project. I discovered the sources for the C++ PyComTest server-dll and the related Python unittest files which seem to me the most appropriate place to implement a test case for this PR.

However, I've also noticed that in CI these tests are not used and that the PyCOMTest sources only contain rather old versions of Visual Studio 6.0 project files. Therefore I'm wondering if the PyComTest server-dll is still actively used locally by the maintainers of this project and if there are probably project files available for some recent version of Visual Studio?

Another possibility to get this compiled would probably be to follow the message in the PyComTest.dsp file which says:

!MESSAGE This is not a valid makefile. To build this project using NMAKE,
!MESSAGE use the Export Makefile command and run
!MESSAGE
!MESSAGE NMAKE /f "PyCOMTest.mak".

However, this would require a version of Visual Studio that can still read the project to export which I don't have.

Please let me know if PyComTest is the way to go.

@geppi geppi closed this Jul 26, 2024
@geppi geppi reopened this Jul 26, 2024
@geppi geppi mentioned this pull request Jul 26, 2024
Open
@mhammond
Copy link
Owner

pycomtest is certainly the right vehicle for testing. I'm afraid I'm a bit swamped at the moment, so it might be a little time before I can give meaningful feedback here.

@geppi
Copy link
Author

geppi commented Jul 31, 2024 •
edited
Loading

I managed to compile the C++ PyComTest server-dll with Visual Studio 2022 and added the method ModifyStruct to the IPyCOMTest interface to implement a unittest for this PR.
Normally a COM interface should not be modified as soon as it is published. However, since this is a test interface purely internal to this project, it is not really published and I do not see a problem in extending the existing interface. If absolutely desired the new method could of course be moved into a new interface but this would also require an additional component to implement the method.

The test case has been added to testPyComTest.py and is limited to the non Dynamic, i.e. Generated case.
In CI the test is not triggered with the current github actions workflow definition.
However, I did run the test locally and it passes.

If the changes of commit 22b28ab are reverted, the test fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing support for record pointers as method parameters of a Dispatch Interface.
2 participants
@geppi @mhammond