Libcanard release announcement

[Pavel Kirienko]

Greetings everyone,

The first release of Libcanard v0.1 has been published today. Grab your copy on Github.

Libcanard is a dependency free, very compact implementation of the UAVCAN stack in C99. It is designed for low cost, deeply embedded applications with limited computing resources and small amounts of available ROM and RAM. It supports NuttX, STM32 (any RTOS and baremetal), Linux SocketCAN, and AVR microcontrollers so far, with more platforms to come.

Special thanks goes to Antoine Albertelli, Michael Sierra, Paul Patience, Matthias Renner, and others.

The project could still use a hand in the following areas:

- Automatic generation of message serialization and deserialization code, like in libuavcan and pyuavcan. Python skills are needed.

- Adding support for new platforms, particularly Atmel SAM and LPC11Cxx.

If anyone is willing to lend a hand to further advance the project, let's coordinate on Github.

Thanks,

Pavel.

[OlliW]

FANTASTIC

I must admit that I had abandoned my interest in UAVCAN because the stepping stones for me had been too high and too many. I'm glad that this project progresses.

I quickly set it up, and could compile the demo with some few changes (which doesn't imply that it would run ;), only compile). Some points are open. I checked the info on git, and from that it seems to me that you have tested that only for Chibios, but not baremetal. I'm doing it baremetal, and on a STM32F103.

Questions:
* usleep(): It compiles fine, but I don't have threads, so this functions looks very suspicious to me. Should I assume that it will work also in a baremetal setting? (I've never used that function before, and actually wasn't even aware it would be available for a non-RTOS arm-C config, so, have no idea how it internally works).
* this is maybe a bug?: you're using assert() in the stm32 drivers, but CANARD_ASSERT() in the canard. I guess I would want the redefined assert be used in both.
* canardSTM32ComputeCANTimings(): that's a stupid question, but I though I just ask to save me some time: it would be

peripheral_clock_rate = 72000000 for 72MHz clock and target_bit_rate = 1000000 for 1MHz can, or 72000 and 1000, or...? * clock_gettime(CLOCK_MONOTONIC, &ts): not available, I guess that's socketcan specific? and I guess I'm supposed to replace it e.g. with a timer, right? (that's at least what  I've tentatively done). This raises the question: would be 16 bit timer OK? would be 32 bit timer OK? or does it really have to be 64 bit? (I would hope for 32 to be OK) * GIT_HASH: that's actually my main question, it is needed in onTransferReceived(), but not defined, and I have no idea what it could be or should be ??? Lastly, I need to find a solution for how to test things. I know, you're going to suggest Babel, but I'm considering this as the very-last option (quite expensive). I wonder: Would it be somehow possible to (mis)use a pixhawk for some basic testing. I mean, could I connect my node to the pixhawk, and then, maybe by using some nuttx shell commands, see if my node is present or even working properly? Cool work! Cheers, Olli

[Pavel Kirienko]

Hi Olli,

Thank you for the feedback!

Please observe that the demo application supplied with libcanard is not intended to show you how to write an application, it is intended to demonstrate how to use the library. This implies that you shouldn't just attempt to run it on your embedded system, especially baremetal, it won't work.

> usleep(): It compiles fine, but I don't have threads, so this functions looks very suspicious to me. Should I assume that it will work also in a baremetal setting? (I've never used that function before, and actually wasn't even aware it would > be available for a non-RTOS arm-C config, so, have no idea how it internally works).

If this function is not defined on your platform (in your particular case, in a bare metal environment, it is not), you should define it yourself. I would propose to make it a simple busy loop.

> this is maybe a bug?: you're using assert() in the stm32 drivers, but CANARD_ASSERT() in the canard. I guess I would want the redefined assert be used in both.

This is a very sensible comment, indeed. It would make sense to unify assertion settings with the core. Would you venture to make a pull request? ;)

> canardSTM32ComputeCANTimings(): that's a stupid question, but I though I just ask to save me some time: it would be

> peripheral_clock_rate = 72000000 for 72MHz clock and target_bit_rate = 1000000 for 1MHz can, or 72000 and 1000, or...?

All units are SI units unless specified otherwise, so Hz. This could also use some extra clarifications - patches are welcome. ;)

> clock_gettime(CLOCK_MONOTONIC, &ts): not available, I guess that's socketcan specific? and I guess I'm supposed to replace it e.g. with a timer, right? (that's at least what  I've tentatively done). This raises the question: would be > 16 bit timer OK? would be 32 bit timer OK? or does it really have to be 64 bit? (I would hope for 32 to be OK)

The function clock_gettime() comes from the POSIX realtime extensions library. On your platform you should define an alternative implementation yourself. The implementation should return the 64-bit number of microseconds passed since an arbitrary moment in the past. 32 bit is not OK because it will be overflowing about every hour, which is not acceptable. You might want to have a look at the STM32 timer driver from libuavcan for inspiration - it is very compact and virtually lock-free.

> GIT_HASH: that's actually my main question, it is needed in onTransferReceived(), but not defined, and I have no idea what it could be or should be ???

It should define the short hash of the current git commit.

Cheers,

Pavel.

--
You received this message because you are subscribed to the Google Groups "UAVCAN" group.
To unsubscribe from this group and stop receiving emails from it, send an email to uavcan+unsubscribe@googlegroups.com.
To post to this group, send email to uav...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/uavcan/6d98fe7a-817d-4db9-aac2-9903e689bd83%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[OlliW]

Hey Pavel,

first, many thx for the detailed answer

Please observe that the demo application supplied with libcanard is not intended to show you how to write an application, it is intended to demonstrate how to use the library. This implies that you shouldn't just attempt to run it on your embedded system, especially baremetal, it won't work.

ok, now you've slowed down my enthusiasm quite a bit ... I'm really puzzled as to why it should not work, and what secret there could be which is missing, as I can't see it. I feel like I really should abandon uavcan for a very long time. I was now really thinking that all crucial pieces are there.
 

> usleep(): It compiles fine, but I don't have threads, so this functions looks very suspicious to me. Should I assume that it will work also in a baremetal setting? (I've never used that function before, and actually wasn't even aware it would > be available for a non-RTOS arm-C config, so, have no idea how it internally works).

 

the funny thing is that it is there (and hence things compile), but I have no idea what it does, as it's linked to one of these obj things which would give me a hard time. Anyway, I use my ordinary delay routine.
 

This is a very sensible comment, indeed. It would make sense to unify assertion settings with the core. Would you venture to make a pull request? ;)

well, I would, if I would know how to use git PRs and would want to learn how to use them, which I both don't ;) What I could do is to post a link to the file(s), which you or someone else could upload, or open an issue with the file(s) copy&pasted - but it's such a simple find&replace thing that I wonder if that efforts are worthwhile. Also, I did found a way to define it outside of the canard.h, so my "solution" might not be satisfying.
 

All units are SI units unless specified otherwise, so Hz. This could also use some extra clarifications - patches are welcome. ;)

thx
 

The function clock_gettime() comes from the POSIX realtime extensions library. On your platform you should define an alternative implementation yourself. The implementation should return the 64-bit number of microseconds passed since an arbitrary moment in the past. 32 bit is not OK because it will be overflowing about every hour, which is not acceptable.

thx
 

You might want to have a look at the STM32 timer driver from libuavcan for inspiration - it is very compact and virtually lock-free.

thx, but is no problem, I just replace uint32_t by uint64_t in my timing stuff

 

It should define the short hash of the current git commit.

thx
 

so far so good, cheers, Olli

[OlliW]

sorry, follow up post

* misspelling correction: It should have been "Also, I did not found a way to define it outside of the canard.h, so my "solution" might not be satisfying."

* usleep() etc: I guess one would want that the user can define that outside, like onTransferReceived() and shouldAcceptTransfer(). If one would do that along the same lines however one would have to pass into the init function also a pointer to it. I did it "simpler" by an extern usleep() definition, which I think is perfectly fine. I thus wonder: Which method should be preferred, i.e. why are the externally defined functions passed as function pointers, and not as external defines? (the latter would look so much "cleaner" to me)

* int vs intxx_t etc: I do not like, at least in the STM32 drivers, that both the int-type and intxx_t-type is mixed, I would prefer it be just intxx_t type. Am I missing a grander picture (compatibility?), or is there any reason not to use intxx_t type?

* uint8_t: In many places it seems to me that a variable defined as uint8_t could be equally well be a uint16_t. I believe to have found that, at least on the STM32F103, using uint16_t often leads to shorter and faster code. E.g. in canardSTM32Transmit() I would not define tx_mailbox and i as uint8_t but unit16_t. Any reasons not to do so?

maybe you're willing to teach me C (if not it's fine :)). I found to constructions which I don't understand.
* in the definition of shouldAcceptTransfer() I find the statement "(void)source_node_id;". What is this good for?
* sometimes you have blocks embraced in {} even though there is no syntactic need for it, e.g. in process1HzTasks(). What effect is this supposed to have?

Anyway, to me it seems that I "only" need a something I could connect the node to to test it.

Thx for all your time and efforts,
Olli

[Pavel Kirienko]

Hi Olli,

> usleep() etc: I guess one would want that the user can define that outside, like onTransferReceived() and shouldAcceptTransfer(). If one would do that along the same lines however one would have to pass into the init function also a pointer to it. I did it "simpler" by an extern usleep() definition, which I think is perfectly fine.

Yes, it is perfectly fine.

> Which method should be preferred, i.e. why are the externally defined functions passed as function pointers, and not as external defines? (the latter would look so much "cleaner" to me)

The approach you outlined is not flexible enough, as it does not allow to instantiate more than one instance of the library.

> int vs intxx_t etc: I do not like, at least in the STM32 drivers, that both the int-type and intxx_t-type is mixed, I would prefer it be just intxx_t type. Am I missing a grander picture (compatibility?), or is there any reason not to use intxx_t type?

You are right, the standard integer types from stdint.h should be used everywhere. I see no pressing need to fix it ASAP, but we would gladly accept a pull request. ;)

> uint8_t: In many places it seems to me that a variable defined as uint8_t could be equally well be a uint16_t. I believe to have found that, at least on the STM32F103, using uint16_t often leads to shorter and faster code. E.g. in canardSTM32Transmit() I would not define tx_mailbox and i as uint8_t but unit16_t. Any reasons not to do so?

The reason not to do that is that it might make the code slightly less intelligible, with a very questionable benefit of possibly seeing the code to be smaller and/or faster in some cases.

> in the definition of shouldAcceptTransfer() I find the statement "(void)source_node_id;". What is this good for?

Explicitly instructing the compiler that it should not complain about this variable being unused.

> sometimes you have blocks embraced in {} even though there is no syntactic need for it, e.g. in process1HzTasks(). What effect is this supposed to have?

Scope limiting. Smaller blocks limit the lifetime and visibility of variables that are defined inside, which reduces the risks of accidentally reusing them unintentionally.

Pavel.

--

You received this message because you are subscribed to the Google Groups "UAVCAN" group.
To unsubscribe from this group and stop receiving emails from it, send an email to uavcan+unsubscribe@googlegroups.com.
To post to this group, send email to uav...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/uavcan/9d2d4cda-c71f-48f2-8717-eb496c3a49d5%40googlegroups.com.

[OlliW]

Hey Pavel,
thx again for your insightful answers

> usleep() etc:

>Yes, it is perfectly fine.

good

The approach you outlined is not flexible enough, as it does not allow to instantiate more than one instance of the library.

ah, thx :)

but coming back again to the usleep() thing, here you would prefer the extern thing or the pointer thing? (for both I would see aesthetic arguments which would favor one over the other)
 

You are right, the standard integer types from stdint.h should be used everywhere. I see no pressing need to fix it ASAP, but we would gladly accept a pull request. ;)

ok :)
 

> uint8_t: In many places it seems to me that a variable defined as uint8_t could be equally well be a uint16_t. I believe to have found that, at least on the STM32F103, using uint16_t often leads to shorter and faster code. E.g. in canardSTM32Transmit() I would not define tx_mailbox and i as uint8_t but unit16_t. Any reasons not to do so?

The reason not to do that is that it might make the code slightly less intelligible, with a very questionable benefit of possibly seeing the code to be smaller and/or faster in some cases.

I see your point. "Intelligible" however has also some personal angle. As regards the questionable benefit of faster code I disagree. In one case I saved 5us just in one function because of using uint16_t instead of uint8_t even though it did byte-oriented stuff (suggesting char of uint8_t)(I was surprised, but this was what it was). On a system with a loop time of 1500us saving 5us by just one function I consider very substantial and worthwhile. I thus made it my habit to always use 16bit over 8bit whenever possible (often it doesn't matter, of course). But I agree, for canard_stm32 it's not much relevant.
 

> in the definition of shouldAcceptTransfer() I find the statement "(void)source_node_id;". What is this good for?

Explicitly instructing the compiler that it should not complain about this variable being unused.

this was my first thought too, but uncomenting it didn't throw a warning (and I have a high warning level) ... seems to be compiler depend
thx for the explanation
 

> sometimes you have blocks embraced in {} even though there is no syntactic need for it, e.g. in process1HzTasks(). What effect is this supposed to have?

Scope limiting. Smaller blocks limit the lifetime and visibility of variables that are defined inside, which reduces the risks of accidentally reusing them unintentionally.

ok, I use that sometimes too, but in longer stuff that is confusing, and not in such short things as process1HzTasks(). I was thinking that it maybe does some memory barrier or whatever, which I failed to know.
thx for the explanation

thx for all your efforts, and help !
Olli

[Pavel Kirienko]

Olli,

> but coming back again to the usleep() thing, here you would prefer the extern thing or the pointer thing? (for both I would see aesthetic arguments which would favor one over the other)

The functionality provided by a sleeping routine is invariant to what part of the application it is invoked from; therefore, an external definition should be used instead of pointers.

> this was my first thought too, but uncomenting it didn't throw a warning (and I have a high warning level) ... seems to be compiler depend

Most compilers require this warning to be enabled manually. On GCC the flag is "-Wunused-variable".

Pavel.

--

You received this message because you are subscribed to the Google Groups "UAVCAN" group.
To unsubscribe from this group and stop receiving emails from it, send an email to uavcan+unsubscribe@googlegroups.com.
To post to this group, send email to uav...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/uavcan/d484d0d9-4b6c-482b-95b0-ab74cf7b9740%40googlegroups.com.

[OlliW]

Hey Pavel

 

The functionality provided by a sleeping routine is invariant to what part of the application it is invoked from; therefore, an external definition should be used instead of pointers.

THX. Also thx for the lesson in C at the github. :)
 

Most compilers require this warning to be enabled manually. On GCC the flag is "-Wunused-variable".

ähm, I think I have that set, as said, I like to use a high warning level, and on other occasions I permanently get this warning. Anyway. I guess I took up enough of your time now.

Thx again so much for all,
Olli


PS: That's maybe the price you have to pay for offering a simpler uavcan implementation :D