-
Notifications
You must be signed in to change notification settings - Fork 636
socketcand Backend for python-can #1140
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
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1140 +/- ##
===========================================
- Coverage 70.49% 70.23% -0.26%
===========================================
Files 79 81 +2
Lines 7679 7920 +241
===========================================
+ Hits 5413 5563 +150
- Misses 2266 2357 +91 |
The |
I have personally not used the deamon yet, but this looks useful. The issue with the test environment should be fixed by now (thanks to @projectgus in #1143). We do not strictly enforce tests or test coverage, but some basic tests are very very welcome! Just a copy of What IS required, however, is at least a small page of documentation, as described in the Developer’s Overview#Creating a new interface/backend. It should document the supported platforms and also the hardware/software it requires (in this case, just socketcand?). A small snippet of how to install the dependencies would also be useful get people started without much friction. |
ping @domologic |
Felix, thanks for your ping and sorry that I've lost this scope. Documentation will follow shortly. |
Felix, please check again. Some doc is also available, you may merge now. |
Felix, please check again |
Why did you add the license in bed0ade? I don't think that is needed and it can be removed. It's already in the project root. |
Ah ok. Its removed now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
One could add tests (see comment) (especially appealing here, as this driver/backend does not need hardware), but it's not required.
Yes, I would also like to see tests here. This would need a kind of "virtual remote station", implementing the same protocol. I hope we can contribute this in future. |
So shall we wait with merging until then? |
No, please merge. We'll contribute tests later, but I can not say when. |
Great, @domologic! Looking forward to more contributions like this :) |
* merge most recent version of socketcand-interface branch * don't change loggine level * do not request python 3.7 * parse < hi > & < ok > messages * Initial upload: Doc for interfaces/socketcand * Python code example was not displayed * indent issues * minor changes * code optimization as proposed by felix * Typo * Monor doc improvement * license information added * reformatted using https://black.vercel.app/ * license removed Co-authored-by: Gerrit Telkamp <g.telkamp@domologic.de>
We are using a Raspberry PI with MCP2515-based CAN-HAT, running a a socketcand as "IP-to-CAN-Gateway damon". For this, we have implemented a new backend.
socketcand uses an TCP-based ASCII protocol to represent the CAN frames. The most relevant advantage against an UDP-based protocol is that the order of messages is guaranteed.
It would be nice if you could merge this into your repository. Please let me know if we have something to add before you can merge the code.