pavel-kirienko
commented
11 months ago Hi @wiboticanders, thank you for the pull request, this is a useful feature. I've been mostly using cannelloni but socketcand is a better tool for many use cases.
I don't like the way it integrates, though. The fact that you had to add multiple special cases to support this interface suggests that either the original PythonCAN wrapper design is unnecessarily rigid, or the new interface doesn't fit the model used in the existing wrapper implementation. I think it is the latter; hence, I think the socketcand support should be split into a separate media layer class, SocketcandMedia. Initially, it can be implemented using PythonCAN, but eventually, we should like to define a native implementation because PythonCAN is not the best library out there, and it is desirable to reduce the dependency on it.
Once SocketcandMedia is implemented, we will want to add a handler for it to the transport_factory:
Added implimention of the socketcand interface. The socketcand interfaces requires additional variables 'host' and 'port' so that it can communicate w a remote computer. This adds those variables as optional, and allows for a socketcand constructor. Socketcand is already functional with python-can, here is the library: https://github.com/linux-can/socketcand. I was not totally sure how you guys preferred this to be implemented, so I have a second implementation that does not add new init vars, and instead adds the new variables to the existing ifacename variable: https://github.com/wiboticanders/pycyphal/blob/noNewInitVars/pycyphal/transport/can/media/pythoncan/_pythoncan.py.