Libcanard v2.0 #182
Libcanard v2.0 #182
Conversation
…he AVL tree in the public API
Preferably, yes. I'm a bit busy at the moment, I'll try to get it done in the next few days but if I'm unable to you can cut it and release 2.0. |
|
And I'll review the rest of this soon as well :) |
| @@ -7,29 +7,23 @@ | |||
| /// ----o------o------------o---------o------o---------o------- | |||
| /// | |||
| /// Libcanard is a compact implementation of the UAVCAN/CAN protocol for high-integrity real-time embedded systems. | |||
| /// It is designed for use in robust deterministic embedded systems equipped with at least 32K ROM and 4..8K RAM. | |||
| /// It is designed for use in robust deterministic embedded systems equipped with at least 32K ROM and 8K RAM. | |||
There was a problem hiding this comment.
This makes me nervous. I'll keep reading but why are we giving up on 4k RAM? For example the Microchip SAMC21E15A is a perfectly modern automotive chip with CAN-FD support that has only 4k of SRAM.
There was a problem hiding this comment.
There are no changes in this PR that are expected to significantly alter the memory footprint. The AVL tree does add a couple of extra pointers per node compared to the linked list but that hardly matters. I updated this description to minimize the chances of submitting the first-time user to the bad experience of trying to squeeze this library into an MCU that is borderline unsuitable. You can make it work with 4K RAM but you really need to know what you are doing, whereas an 8K chip provides some laxity in memory allocation.
| #include <assert.h> | ||
| #include <string.h> | ||
|
|
||
| // --------------------------------------------- BUILD CONFIGURATION --------------------------------------------- | ||
|
|
||
| /// Define this macro to include build configuration header. |
There was a problem hiding this comment.
would it make sense to add something like:
#if CANARD_VERSION_MAJOR != 2
# error "Wrong canard.h"
#endifI'm sure it wouldn't compile, ultimately, but this error is easier to figure out then trying to parse out the random failures including the v1 header by accident would cause.
There was a problem hiding this comment.
Do you mean putting this check inside your build configuration header? It is user-provided so one can put there whatever is desired.
| /// just pointers, but it would push the size of this instance from about 0.5 KiB to ~3 KiB for a typical 32-bit | ||
| /// system. Since this is a general-purpose library, we have to pick a middle ground so we use the more complex | ||
| /// but more memory-efficient approach. | ||
| struct CanardInternalRxSession* _sessions[CANARD_NODE_ID_MAX + 1U]; | ||
| struct CanardInternalRxSession* sessions[CANARD_NODE_ID_MAX + 1U]; ///< Read-only DO NOT MODIFY THIS |
There was a problem hiding this comment.
Regarding @thirtytwobits statement
I'll keep reading but why are we giving up on 4k RAM?
Would utilizing CAVL here save enough memory to support 4KB RAM targets?
The maximum number of sessions is 128 thus would give you at most 7 hops in a binary tree.
Didn't look into the specifics of CAVL yet, but I suppose it's more memory efficient then allocating 128 pointers.
There was a problem hiding this comment.
The maximum number of sessions is 128 thus would give you at most 7 hops in a binary tree.
10 hops in the worst case (the upper bound is a bit more involved than log2(size))
Didn't look into the specifics of CAVL yet, but I suppose it's more memory efficient then allocating 128 pointers.
This is true. From the time complexity perspective though, CAVL is less desirable because O(1) array lookup is much easier to analyze and validate than O(log n) tree search. We can certainly go this way but should we really?
Adds a build option `CANARD_CRC_TABLE` which replaces the runtime-computed CRC with a static table. This option adds ~512 bytes ROM but reduces computation time significantly. Fixes #185 CI: disable sonarcloud on forks Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Fixes #143 Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
Adds helper API functions for configuring CAN controller hardware acceptance filters. Closes #169 Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
|
Kudos, SonarCloud Quality Gate passed! |
See the changelog in the README for a brief overview of the changes. The most interesting changes are two:
@coderkalyan do you want to include #171 into this release or should we wait for v2.1?