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.

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

tests: support windows #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JPHutchins
Copy link

@JPHutchins JPHutchins commented Jun 7, 2022

More of a question than a PR. I have used the tool com0com-2.2.2.0 to create a pair of virtual com ports and do the loopback test provided in the tests module. Tomorrow I will test with a pair of FTDI USB adapters and report back.

I am currently working on a project that must run under Windows - I would love to help get this fully supported. I do not understand the note here since the tests pass:

Posix platforms only, Python 3.5+ only.
Windows event loops can not wait for serial ports with the current
implementation. It should be possible to get that working though.

Perhaps this is a good place for me to start?

Regarding flush (in regular pyserial), I'm not sure that I understand this comment:
https://github.com/pyserial/pyserial/blob/31fa4807d73ed4eb9891a88a15817b439c4eea2d/serial/serialwin32.py#L345-L349

    def flush(self):
        """\
        Flush of file like objects. In this case, wait until all data
        is written.
        """
        while self.out_waiting:
            time.sleep(0.05)
        # XXX could also use WaitCommEvent with mask EV_TXEMPTY, but it would
        # require overlapped IO and it's also only possible to set a single mask
        # on the port---

My reading of the documentation is that lpEvtMask is a 16-bit value so we could listen to all, for example 0xffff and in this function just mask for 0x0004 to check if the buffer is empty. I don't have much experience with these Windows APIs but my hope would be that WaitCommEvent blocks until an event occurs and that it would avoid the busy loop altogether.

Regardless of the number of events listened for it would look something like:

while (p_lpEvtMask & EV_TXEMPTY) == 0:
    success = win32.WaitCommEvent(self._port_handle, p_lpEvtMask)

Very excited to learn more!

@JPHutchins
Copy link
Author

I have confirmed the small unit test with two USB UARTs on Windows 10.

@JPHutchins
Copy link
Author

OK, I think I see that await writer.drain() is returning prematurely. I am however able to block on the flush() method of the transport. The FlowControlMixin in the asyncio.streams module has a _drain_helper() function that is hard to follow but my initial impression is that this might be the function that is returning instead of waiting for Windows' serial write buffer to empty.

@JPHutchins
Copy link
Author

JPHutchins commented Aug 21, 2022

The following is working for me in serialwin32.py:

Setup listener for EV_TXEMPTY in _reconfigure_port():

win32.SetCommMask(self._port_handle, win32.EV_ERR | win32.EV_TXEMPTY)

And replace flush() with:

        if self.out_waiting == 0:
            return

        EvtMask = win32.DWORD(0)

        while (EvtMask.value & win32.EV_TXEMPTY) != win32.EV_TXEMPTY:
            win32.WaitCommEvent(self._port_handle, ctypes.byref(EvtMask), ctypes.byref(self._overlapped_write))

@JPHutchins
Copy link
Author

OK, combined with the modification above we can create an awaitable Future by listening for the Windows event.

    async def write_complete(self):
        await self._write_complete_future
    
    async def send(self, data):
        self._register_write_complete()
        self.write(data)
        await self.write_complete()
    
    def _register_write_complete(self):
        if platform.system() == "Windows":
            self._write_complete_future = self.loop._proactor.wait_for_handle(self._serial._overlapped_write.hEvent)        

And with that we have a serial send() method that is an awaitable coroutine returning once Windows has emptied it's serial buffer to the transport.

I think I will move along to PR pyserial instead since it should support asyncio directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant