nvda icon indicating copy to clipboard operation
nvda copied to clipboard

hwIo.base.IoBase relies on braille._BgThread

Open LeonarddeR opened this issue 3 years ago • 4 comments

Is your feature request related to a problem? Please describe.

There is a reference from hwIo.base.IoBase to braille._BgThread that we need to get rid of at some point. This will probably be API breaking, but it would really help if we could make IoBase independent of the background thread that it runs it APCs on.

Describe the solution you'd like

Provide a callable to the constructor of IoBase where it can throw its APC at.

LeonarddeR avatar Sep 12 '22 17:09 LeonarddeR

Ideally hwIo and braille display drivers should not know about the braille display background thread at all, while they still should be able to queue an APC to a thread they don't know. I guess this is almost impossible. Even if we pass a callable to the constructor of Iobase, the braille display driver needs to know about the callable and thus about the thread. Pretty sure @jcsteh pondered about this issue for ages when he implemented async I/O.

LeonarddeR avatar Sep 12 '22 17:09 LeonarddeR

it would really help if we could make IoBase independent of the background thread that it runs it APCs on.

I guess the first question I'd ask is: aside from code coupling, why? I'm sure you have a good reason, but I think it helps to think in terms of use cases first before making design changes.

If this were just about code coupling, I might suggest we move the background thread to hwIo or something so that it's at least not braille specific. From a design perspective, what we're saying then is, "braille needs to use the background IO thread to do some i/o stuff," rather than, "hwIo needs to use the background braille thread to do some i/o stuff," which is an admittedly weird relationship. That is, the relationship should be clearly that braille depends on hwIo, not the other way around.

I don't know that providing a callable really solves this, as you say. If anything, it might make things slightly worse because now drivers are aware of this internal implementation detail (or maybe they aren't; I forget who constructs IoBase).

jcsteh avatar Sep 13 '22 04:09 jcsteh

it would really help if we could make IoBase independent of the background thread that it runs it APCs on.

I guess the first question I'd ask is: aside from code coupling, why? I'm sure you have a good reason, but I think it helps to think in terms of use cases first before making design changes.

I'm intending to write a speech synth driver that relies on I/O. the hwIo module would be perfectly suitable for that, apart from its dependency on braille.

If this were just about code coupling, I might suggest we move the background thread to hwIo or something so that it's at least not braille specific.

That makes sense. Note however that the thread isn't i/o specific either, i.e. it also processes APCs. bdDetect also relies on the thread.

I don't know that providing a callable really solves this, as you say. If anything, it might make things slightly worse because now drivers are aware of this internal implementation detail (or maybe they aren't; I forget who constructs IoBase).

In the end, it's the driver that constructs IoBase due to constructing one of its subclasses.

LeonarddeR avatar Sep 13 '22 05:09 LeonarddeR

Note however that the thread isn't i/o specific either, i.e. it also processes APCs. bdDetect also relies on the thread.

That's true. That said, while it's a bit of a stretch, the main reason we queue APCs to that thread is that they generally do i/o in some way and we don't want that to block the main thread. So, even if they aren't strictly i/o reads/writes, most (all?) things we queue to that thread are at least loosely related to i/o. If wxWidgets used an alertable wait in its event loop, we might not have used a background thread at all, especially if that meant we could use asyncio (though that's obviously a much bigger topic).

All of that said, I just realised bdDetect at least does some blocking stuff on that thread, so using the same thread for synths probably wouldn't work so well. Hmm.

jcsteh avatar Sep 13 '22 05:09 jcsteh