vendor, io: fix missing i_domain and o_domain arguments when specializing

[richardeoin]

Platforms implement the get_io_buffer method to provide vendor-specific implementations of io.Buffer, io.FFBuffer and io.DDRBuffer. Currently several platforms use isinstance to match the type of buffer, then construct a child class of the relevant buffer and return this object. Unfortunately these platforms miss the optional i_domain and o_domain arguments when calling the constructor, thus placing the vendor-specific specialisations in the default sync domain even if an different domain is specified in user code. This is the bug.

Only io.FFBuffer and io.DDRBuffer are affected as these are currently the only buffers with optional arguments in the constructor signature.

This could be fixed by modifying each vendor implementation to pass the optional arguments as required. That might look like this:

DDRBufferSpecialised(buffer.direction, buffer.port, i_domain=getattr(buffer, '_i_domain', None), o_domain=getattr(buffer, '_o_domain', None))

However this pushes more complexity into the vendor implementation and requires maintenance across an ever increasing number of vendor files.

This PR implements a factory method called create_from on the base class. This method is used in the vendor implementations to correctly create the specialised classes.

This is a fix for the new lib.io components, so mentioning the tracking issue here #1210

[richardeoin]

Arguably this method should also be implemented on Buffer for consistency. Very happy to add that after review.

[whitequark]

I've taken a quick look at your PR. Offhand, there are two concerns:

• It adds a new public API, which isn't something we do without at least some upfront design.
• It raises questions about the design of the existing API as well.

I think this PR should probably be a blocker for the 0.5 release, but other than that we'll need more discussion of the direction first. I'll only be able to get around to that after handling some of the other release blockers first.

[richardeoin]

Indeed this was a sharp edge on the existing API (as viewed from the vendor implementation) that I have sanded down with this PR, but it may be that a different solution avoids this completely. I can also write this up as an issue (without the solution) if that is easier for the triage process?

[whitequark]

The PR is fine.

[richardeoin]

Awesome that this is fixed now 🎉