AgIsoStack-plus-plus icon indicating copy to clipboard operation
AgIsoStack-plus-plus copied to clipboard

NTCAN hardware layer

Open Y-Less opened this issue 3 years ago • 4 comments

I wanted to add support for this virtual CAN bus:

https://esd.eu/en/products/can-sdk

I know there's a basic virtual CAN already in the library, but it doesn't work between processes.

This is only a very basic first pass at the code (and clearly copied almost wholesale from the PCAN implementation), just here for any early comments (I've not even tested it yet). They do claim that their library is cross-platform, so there may be a way to support Linux in here as well. You may notice that the bundled .lib files are in sub-directories instead of using the existing _x86 and _x64 naming convention on other libraries. This is because the include itself, copied from their SDK, looks for a file named exactly ntcan.lib via #pragma.

Y-Less avatar Mar 03 '23 02:03 Y-Less

Thanks for the comments. Fortunately most of those seem fairly trivial. As for the main point - basic testing has at least confirmed it works.

Y-Less avatar Mar 03 '23 14:03 Y-Less

So I'm not sure what the best way to deal with removing the explicit width types in, since I used those to match what the NTCAN API expects. If a function expects uint64_t I don't want to pass long long just in case... But maybe I should and just accept errors when they don't match. On the other hand the explicit width types are not exposed outside the plugin at all, and since the API itself has them this code isn't introducing any new incompatibilities that aren't introduced by the API itself. I've prefixed them with std:: anyway though.

0 | NTCAN_20B_BASE was me trying to be explicit. It is address 0 with the extended IDs flag, rather than just the flag alone. Yes it is pointless from a maths point of view, but I thought it was more helpful from an understanding point of view. I added a comment to this effect.

As for the inclusion of the library directly, I did that because the PCAN one did and I was just trying to emulate the existing style. However, since you need the driver installed to use this hardware backend anyway, and that includes the SDK with the latest version of the includes and libs, I don't think much is lost by removing it entirely and not worrying about the licenses at all. I can even rebase and purge the copies from git history.

Y-Less avatar Mar 04 '23 04:03 Y-Less

I've rebased and removed the copies of the ntcan library. Now you just have to link it correctly.

Y-Less avatar Mar 09 '23 08:03 Y-Less

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Mar 09 '23 17:03 sonarqubecloud[bot]

NTCAN added in #504

ad3154 avatar Dec 29 '24 20:12 ad3154