Separate drivers and stack

[martinwag]

Separating drivers and the stack is a good solution to get consistent working builds

see #105 (comment)

With the stuff I merged in commit 8b01062 I think I broke all drivers except driver template anyway...

[geoffrey-vl]

Do you wish to also erase the currently available drivers? That would imply having all demo apps updated. If so we could also go ahead and delete the driver directories from this repo.

[martinwag]

For it to be clean, this would be the way to go.

However, I can't do more than just erase them. I neither have the time nor the hardware/compilers to move them out and get them working again...

I also have no idea if all drivers are up to date with their manufacturers environment (except the ST one which is really outdated).

[JuPrgn]

That would be nice to get a repository CANopenDriver created by CANopenNode with multiple maintainers instead of drivers everywhere. I may help with PIC drivers.

[geoffrey-vl]

Well, there are a lot of question that come to my mind here.

Typically we see that someone enables the driver for one kind of MCU, and afterwards there are no updates on that driver, or just very few. The reason could be different kind of things:

• it was just a test or prototype
• it was an implementation used in production, but its stable and doesn't require upgrading to the latest
• someone made it work professionally but doesn't work on the project / company any more

Would you allow these drivers to be merged, knowing they'll not be updated? Furthermore, it doesn't happen too much that we have 2 projects using the exact same MCU, while drivers are actually tightly bounded to the MCU. Will a maintainer work on updating the driver? For example: mbed-os is updated once each few months, I don't want to be the guy to updating/testing that driver :)
How to check if the implementation actually works? Compliance tests? How can maintainers be sure the pull in a driver for MCU that he doesn't own?
Since an implementation would be typically bound to a project, aren't drivers almost by definition outdated? Do we find that a problem?

On the other side, I recon having one repo to rule them all is a good way to not spread the implementations over the entire internet. Links get broken for example (= code rot?)... How can we handle that? What about having mainline drivers, which have proven their usage (through compliance tests)?

[JuPrgn]

In my opinion the drivers provided with the stack should be considered as examples for a family of microcontrollers.
They should be updated when we add features such as the initialization of new variables of the stack...
Anyway I do not plan to update and test all drivers periodically I think rather the maintainers should merge patches commited by users of these drivers.

I am not sure outdated drivers are a problem if someone want to use an 'outdated driver' it is easier to start from something existing and update/compare it based on the driver template than starting from zero.

[geoffrey-vl]

FYI: I just pulled in latest master and I was able to use the stack perfectly using the drivers I made for CANOpen node v1.0.

So maybe we can take out the current drivers and put them in a separate repo that has only these legacy drivers: CANOpenLegacyDriver. That repo should be read only. What do you think?

[Bartimaeus-]

It seems to me that separating the drivers from the stack into a CANopenNodeDriver repo like @JuPrgn mentioned and having folders for each of the versioned releases of the stack would make driver management a lot easier. This would have a number of benefits:

• Drivers for older versions of the CANopenNode stack can still be maintained/added independent of the other releases

• It becomes easy to see what changes are needed for a particular driver to support a newer released stack version (and those changes can be managed independently based on who is working on them)

• Updates and new features to the CANopenNode stack can be added at any pace without breaking all of the drivers

Whenever someone submits a driver for a version of the stack it can be placed under the appropriate folder in the repo and there can be a list describing what level of compliance/validation testing has been performed for that driver (self-reported and officially recognized testing). I don't see a need to enforce strict criteria about driver submission as long as the status is tracked.

[geoffrey-vl]

Using folders for versioning your drivers... so why use git altogether?
In that case I'd rather branch the drivers repo according to CANOpen release.
At least that's how I've seen it being done with mbed-os and it example applications, or for example with yocto releases and its various meta-layers.
If you want to to stick with an older version of CANOpen, you can still update the driver within that specific branch.

I'm curious on how you'd do the compliance/validation. Basically the repo maintainer(s) could easily pull in new drivers based upon that report. Maybe you could further elaborate how the validation process looks like?

[Bartimaeus-]

@geoffrey-vl I think that sounds like a fine way to could achieve the goals I mentioned above. A link in the CANopenNode release notes to the appropriate driver branch to make it easy to locate would be nice.

For compliance and validation, the official test plan is specified in CiA DSP 310-1. Anyone who's working for a company that's a CiA member can also get a free copy of the official compliance test tool (that's what I'm using). If we can write out a list of what functions the stack relies on the drivers/application for, I suspect that a small subset of the tests could be used to test the drivers for compliance (most of the compliance tests aren't going to be driver specific). Maybe someone can put together an open source tool for this?

[odesenfans]

I'm attempting to tackle one of the obstacles to properly splitting the code: the redefinition of CO_driver.h for each driver.

Most of the functions are common to all drivers and must have the same API since they are used in target-independent code.

What remains are the definitions of synchronisation primitives. These should probably be defined as functions rather than macros as to avoid having to include target-dependent header files. Furthermore, synchronisation primitives feel awkward, I need to investigate but I'm 99% sure they could be replaced by atomic variables. Same for the various uses of volatile variables, they should be replaced by atomics!

I suggest to:

• either define the CO_LOCK_ and CO_UNLOCK_ as functions instead of macros, or keep them as CPU-dependent macros and split them from the actual drivers.
• remove the SET_CANrxNew, CLEAR_CANrxNew, etc macros and update the code to use a compiler-dependent definition instead. GCC provides builtins. Furthermore, they're officially supported by the language since C11 and equivalents. They're necessary for single-core MCUs. Actually they're only defined for socketCAN code, so there's definitely a driver maintenance issue there as well!

The last remaining bit is the macros to define endianness (CO_LITTLE_ENDIAN and CO_BIG_ENDIAN). They are currently defined driver by driver, but could be defined for each supported CPU.

EDIT: I missed the custom fields in CO_CANrxMsg_t, CO_CANrx_t and CO_CANtx_t. These seem to be HW-dependent, so either we remove them and rely on a default implementation to compile, or we keep the definitions in the main repo.

EDIT 2: Could we list the drivers that are used by at least someone?

What do you think?

[martinwag]

Hi,

Syncing took me quite a while to find a solution back then.

• This is not a gcc-only project.
• This is a bare-metal project (No Linux/Posix, Freertos, EMBOS, MBED, ...).
• Embedded compilers are ---very--- old sometimes, so C99 is actually an optimistic requirement.
• Proprietary embedded compilers - see before, but sometimes not compatible to recent standards in current versions.
• I don't want lock/rxnew to be functions as they are run in IRQ context, where I can't decide how critical overhead is.
• Small microcontrollers don't have pipelining or compiler code re-ordering, so don't need the sync functions anyway. With macros, this leads to optimization without the compiler doing anything.
• I've tried inline functions, but this broke on the first non-gcc compiler I tried (in a way that code compatible with the second compiler is incompatible with gcc...)
• If you wantsync to be a function, you can do this with the macro. This doesn't work the other way round.
• ...

So, yes, I think C11 "atomic" is the best solution. But unfortunately not available here. I didn't find any other solutions, so going "you have to do that yourselve" was my best idea. I'm open to better solutions.

I think the current driver design isn't that bad. It's mostly that drivers bring code to the stack that can't really be maintained properly.

regards,
Martin

[odesenfans]

I understand your point. We can definitely keep the synchronisation primitives as macros. My idea was that they are more of a compiler-specific issue than a target-specific issue. We could for example have a compiler_gcc.h file and a compiler_xc16.h file that get selectively included depending on the target. This will remove duplication for all targets using the same ISA.

My second point is that a lot of functions currently declared in each CO_driver.h file should be declared at one place project-wise. They are used in target-independent code and must be defined the same for each target ABI-wise. I started a branch to try this out: https://github.com/odesenfans/CANopenNode/blob/common-header-for-drivers/CO_driver.h

[CANopenNode]

I would like to separate the drivers, because it is not possible to maintain the code, for which the maintainer does not have a hardware, software tools, supplementary files, etc. I can currently maintain only PIC and Linux.

I want keep CANopenNode clean, but there are many different implementations on many different hardware, by many different developers. I want to put the responsibility for the specific (group of) hardware to the specific (group of) people.

If someone implements some exotic device and want to share it or collaborate to others, then it's best to host it on his place. CANopenNode/wiki is public editable and he can put a link there.

If device is more widely used, like PIC or Linux or STM or LPC, etc, then I recommend to put the project on this place. CANopenPIC and CANopenSocket are already here, I can add repository for other devices (or better, group of devices, or for specific tool like MBED) and specify collaborator(s). I don't expect much, basically project should contain driver files, optionally very short demo, some description and link to CANopenNode. I recommend adding CANopenNode as a git submodule, it automatically keeps track on version used.

I moved 'Microcontroller support' section from README file into wiki and add some text into README. For now, just to be on one place.

---

CO_driver.h is already extracted, thanks to @odesenfans. I will try to copy driver files into CANopenPIC and do some tests. I can also add @JuPrgn as a second maintainer.

I lost track on CANopenSocket a little since my last commit. I would like to leave the transfer of driver files to @martinwag. Or at least I need some advice.

For other devices I have no fresh information (except @WillyKaze contributed LPC1768 with MBED). If someone is using driver files, please make simple demo project with driver files included.

I don't know, how it is with MBED. Mbed-os RTOS + STM32 F091RC by @geoffrey-vl is already available from wiki. Could other devices be used too?
Similar is Zephyr RTOS by @henrikbrixandersen, also mentioned in the wiki.

[geoffrey-vl]

Hi,
I made my mbed implementation specific for STM32. While I did try to use as much of the mbed api's, some functionality is really device specific and calls device specific code that sits underneath mbed-os. I don't own any mbed boards other than those of STM, that's why I didn't try to merge the LPC1768 code. However, I did look at it and used it as a reference. If it would become some middle ground for all mbed boards I guess we could build upon that.

I like the idea that CANopenNode is really just being a library that contains logic. I just want to point out one thing. Those driver repo's will still have the same problems that we have now: we will have people adding device specific code for a specific mbed-os release, over time API's change, drivers will not be maintained and code will rot. So in the end we've just moved our problems (and responsibilities) out of CANopenNode into CANopenNode-driver-**** . I think that this problem is very hard to cover, the platforms are too divers and there is way too little people involved. I think we should start as you suggested and act. We can review these changes over time and see where it leads us, but I think it in the end we will gain portability and acceptance.

One other thing, it would be nice if the EDS editor became an official part CANopenNode. I have no idea if that is somehow legally possible, or even wanted by all parties.

[CANopenNode]

I started using Linux when I started developing CANopenSocket and now I just like its philosophy. It says: 'Small is Beautiful', 'Each Program Does One Thing Well', etc. Maybe that's the reason, why I like splitting the project into smaller parts and make a good interface between them. Similar is with CANopen: many smaller "smart" devices, than single big central controller.

I do not worry about the problems, split repositories may have. Good implementations will live on, people will easier maintain them, they won't always have to wait for the 'CANopenNode maintainer' to move.

I think, the most important is, to keep a good track to available implementations. I think, public editable CANopenNode/wiki is perfect for this. Maybe we can make things more clear there. We can make a table. Also supported features can be specified more precisely, CANopenNode version can be noted, maybe links to some interesting examples or forks can be added, etc.

Second thing, I already edited CANopenNode/README.md and recommended libedssharp as OD editor. But for the reasons, I described above, I prefer to keep it as a separate project. However, we can move the project here, besides CANopenNode, but currently this doesn't make a big difference.

And third thing, I have some more time now and I will try to make new OD interface in next few months + some other things.

[CANopenNode]

I pushed new branch: split-driver.
files from drvTemplate were moved to

• stack/CO_driver.h
• example/CO_driver_target.h
• example/CO_driver.c
• example/eeprom*

Code compiles well. CO_driver.h is much better organized, documentation is much updated. Some code in other files in 'stack' directory is updated. CANopen.h is excluded from doxygen documentation.

[geoffrey-vl]

Next will you move all drivers away from this repo?

[CANopenNode]

I do not need to hurry with removing them.

Now I'm going to update the PIC microcontrollers (copy drivers to CANopenPic project).

Then CANopenSocket (I would like to hear some advice from users, because I haven't been here for some time).

After that I plan to:

• tag master branch, (v1.3)
• remove other drivers and merge the split-driver branch into master (v2.0)
• List removed drivers in wiki and add them a link to v1.3
• Mention removed drivers in README.md

So basically, they will always remain in the repo, they will just be removed from the master branch.

The fact is, I have no information, if and where other drivers from this repo are used. Except those listed in Wiki.

And also, CO_driver.h is now more or less the same as before, except it is more clear. So it shouldn't be hard to make simple repo(s) with example for still relevant drivers. I'm also ready to open project for specific (family of) drivers here and specify the collaborators.

[geoffrey-vl]

One thing: most likely changing the OD is a big change that will break libedssharp. I'm not sure if you agree but I'd call that a mayor change. I assume we don't take too many steps at once and call that V3.0?

[CANopenNode]

Yes, changing the OD will be a mayor change. libedssharp will probably follow the change. Until new OD won't be stable, it will be in separate branch.

[robincornelius]

On Tue, 21 Jan 2020 at 11:13, Janez ***@***.***> wrote: Yes, changing the OD will be a mayor change. libedssharp will probably follow the change. Until new OD won't be stable, it will be in separate branch.

It will be another exporter added to the export options, I'll leave the existing exporter alone. It will be interesting to see what needs changing as my goal is to use the native XDD and EDS fileformats for saving the OD. Currently there are a couple of "extensions" i have added in the XDD the main one is the COS Notify flag, there does not appear to be anything in the XDD format to permit such a field. And i'm sure there will be other interesting problems to solve. Time permitting once an idea for the new OD is in git i'll try to make a start on the new exporter as well. Robin

[CANopenNode]

CANopenPIC is now completely updated.

[CANopenNode]

Just a note: Branch split-driver should be used for updating own drivers.

[CANopenNode]

Before I continue with CANopenSocket I have a question (for @martinwag) about two drivers for it: original socketCAN and neuberger-socketCAN. I didn't check the latter yet.

I suppose, socketCAN is now outdated (my fault, I wasn't reading the messages). So neuberger-socketCAN can be a replacement for socketCAN, and old socketCAN has no advantages over newer neuberger-socketCAN.

If that is so, I would like to drop old socketCAN driver. Is it OK?

Are there any other suggestions about CANopenSocket?

[geoffrey-vl]

I'm still relying on CANopenNode's CANopenSocket for testing CANopen modules. So I'd like to know if it gets removed that the replacement works as a drop-in replacement?

[CANopenNode]

One more thought: CANopenSocket is quite wide project. There may be other projects based on Linux driver. Does it make sense to make an exception and keep the Linux driver part of the CANopenNode? In that case I would move ./stack/neuberger-socketCAN/ into ./socketCAN/

[martinwag]

The old socketCAN driver has some bugs/undefined behaviour compared to my one, so I would recommend replacing it.
Please, have a look at CANopenSocket first, I think there was a incompatibility. I didn't look at this further when I added the driver, should be only something minor...

[martinwag]

I've had a short look at the differences between the two socketCAN drivers

• I don't have OD storage (and don't need it). Maybe just copy from your driver?
• Function names are a bit different (Task <-> Thread is a major difference in Linux).

[CANopenNode]

Yes, I already copied OD storage.
I will have to re-think mainline (threads), I will probably include main.c near the driver, just for completeness.

[CANopenNode]

I have a question about neuberger-socketCAN:
There are some log_printf() functions and some macros declared in headers, which seems to be custom:

#if defined CO_DRIVER_ERROR_REPORTING && __has_include("syslog/log.h")
  #include "syslog/log.h"
  #include "msgs.h"
#else
  #define log_printf(macropar_prio, macropar_message, ...)
#endif

...
log_printf(LOG_DEBUG, DBG_CAN_SET_LISTEN_ONLY, CANerrorhandler->ifName);

I would like to add at least fprintf(stderr, ..., but I can't find the macros like DBG_CAN_SET_LISTEN_ONLY, etc.

[martinwag]

Yes, they are custom. Our product uses the linux syslog for storing errors, as we don't have a terminal open at runtime. I did not put much thought into how to make this universal at the time that I wrote it, except for someone else not to get compile errors.
I'm not really sure how to solve this, as I'm required to

• store all messages of my entire app in one single file (means I'm unable to just add another file to the driver)
• have the messages in German...

Do you have a good idea?

For reference, those are the macros:

#define CAN_NOT_FOUND             "CAN Interface \"%s\" not found"
#define CAN_INIT_FAILED           "CAN Interface  \"%s\" Init failed"
#define CAN_NAMETOINDEX           "Interface \"%s\" -> Index %d"
#define CAN_SOCKET_BUF_SIZE       "CAN Interface \"%s\" Buffer set to %d messages (%d Bytes)"
#define CAN_BINDING_FAILED        "Binding CAN Interface \"%s\" failed"
#define CAN_ERROR_FILTER_FAILED   "Setting CAN Interface \"%s\" error filter failed"
#define CAN_FILTER_FAILED         "Setting CAN Interface \"%s\" message filter failed"
#define CAN_RX_SOCKET_QUEUE_OVERFLOW "CAN Interface \"%s\" has lost %d messages"
#define CAN_BUSOFF                "CAN Interface \"%s\" changed to \"Bus Off\". Switching to Listen Only mode..."
#define CAN_NOACK                 "CAN Interface \"%s\" no \"ACK\" received.  Switching to Listen Only mode..."
#define CAN_RX_PASSIVE            "CAN Interface \"%s\" changed state to \"Rx Passive\""
#define CAN_TX_PASSIVE            "CAN Interface \"%s\" changed state to \"Tx Passive\""
#define CAN_TX_LEVEL_ACTIVE       "CAN Interface \"%s\" changed state to \"Active\""
#define CAN_RX_BUF_OVERFLOW       "CAN Interface \"%s\" Rx buffer overflow. Message dropped"
#define CAN_TX_BUF_OVERFLOW       "CAN Interface \"%s\" Tx buffer overflow. Message dropped"
#define CAN_RX_LEVEL_WARNING      "CAN Interface \"%s\" reached Rx Warning Level"
#define CAN_TX_LEVEL_WARNING      "CAN Interface \"%s\" reached Tx Warning Level"

and log_printf is a wrapper for syslog() https://www.gnu.org/software/libc/manual/html_node/Syslog-Example.html

Edit: erros in macros

[martinwag]

Of course, we can just add the necessary files to the driver and I keep a changed version in my local repo. I don't really want the driver to be more complex than necessary because of company internals...

[CANopenNode]

I think, log_printf() functions are just fine as they are. And there should be an option for custom log principle and custom message contents.

I will just add your defines so they could be optionally selected. Do you also have defines starting with DBG_, for example DBG_CAN_SET_LISTEN_ONLY?

[martinwag]

I'm going to have a look at this as some of the multi interface stuff is broken since changing driver API anyway.

[CANopenNode]

If you wait a few days. I will write main.c based on neuberger-socketCAN driver and make some minor changes in the driver to suit the API.

Then I will also move files from project CANopenSocket/canopend (master / DS309 files) into CANopenNode/309. And make it universal, better organized on one place.

I think, there will be only simple changes, so there should not be much job to fix broken things in custom project.

If you will take a look into reorganized code and make it suitable to your needs, you can also reorganize if you like. I think, code is universal enough and it should also be easy to include custom stuff. I prefer, if we all use the same base code, even if it is a little more complex. People often verify the code and warn if find something strange.

[martinwag]

I've pushed the current version to master branch. This includes all log messages and various fixes to be compatible with master. I'm not going to merge into split-driver currently.

Feel free to remove the "neuberger-" prefix, it's only there because at the point I started socketCAN was already taken :-)

I would prefer to keep the CANopenSocket / canopend stuff in it's own repo. As it's currently implemented, it's more like a debugging tool and a demo application for how to use CANopenNode than production-ready code.

[CANopenNode]

Thank you. I will merge master into split-driver. I will also try to document changes in doc/changelog.md.

"neuberger-socketCAN" allready steal the name "socketCAN" :-)

I would like to move CANopenSocket / canopend, because all files except main.c are actually implementation of the DS-309-3 standard (CANopen access - ASCII mapping). And those files could be used also on microcontroller via serial interface.

[martinwag]

I don't think the canopend/309 stuff will run on a simple microcontroller. We have dependencies for half a POSIX system in there, so you need at least one of the more advanced RTOS. You also can't use POSIX stuff on Windows, if anyone cares...

[CANopenNode]

I will check again, how complicated it is. I intended to separate socket and thread stuff, which is specific for Linux. Then there only is a blocking function with input as a command string which writes out the answer string.

[CANopenNode]

Between the v1.2 and master there were some commits, which changed the driver (Single project-wide header and void pointer for the CAN device address). But they were not systematic enough and there were some bugs.

When I started with split-driver branch, I first reverted messy commits and used original driver code. Then I made own "Single project-wide header". I leaved "void pointer for the CAN device address" but made a full check and comparison with the original code.

I think, some parts of current master branch is quite messy, especially drivers, I'm more focused on split-drive now. I will verify your fixes in current master and will also update the split-driver.

Please also note some bugs found in original code, which were fixed in split-driver, but not in master:

• BUG in CO_HBconsumer.c Possible BUG in CO_HBconsumer.c #168
• BUG in CO_HBconsumer_process() there is argument timeDifference which is set to 0 inside for loop, but should not be.

[CANopenNode]

I must change my opinion: I think, current master branch from v1.2 is not so messy, there were some very good contributions. Also O. had had some good ideas, just his contributions were not fully thorough. However, I think stack is OK now.

[CANopenNode]

I'm slowly advancing with the stack. Most updates are in the branch split-diver.

It is rearranged now (see #161), but fundamental code remained more or less the same. I Allowed myself some more freedom for changes (according to my taste). I hope, code is better organized and documented and will be usable. It is tested to some extent and it seems to work fine on Linux and PIC microcontrollers.
I know, that people, who are used to current code will need some effort to switch to renewed. Also existing projects, well tested, will probably stay with existing stack for some time.
I plan to merge split-driver branch into master soon. But I will keep existing master branch for some time to keep it available for updates. I will just rename breanch to "old-master".
This will be mayor release, v2.0. Then I plan New Object Dictionary (#162), it will be v3.0.

Please take a (quick) look into a new code. Some changelog info is here: https://github.com /CANopenNode/CANopenNode/blob/split-driver/doc/CHANGELOG.md. If there are some ideas or proposals, please share it. Even if some existing principle will be broken, if there is improvement, now it is the time for changes. I just don't want to change some taste specific things, like reformatting all the code.
Device Support information is now part of the stack. I plan to remove info from wiki. See https://github.com/CANopenNode/CANopenNode/blob/split-driver/doc/deviceSupport.md

Linux socketCAN driver: it is now part of the stack and is copy of "stack/neuberger-socketCAN". There are some minor updates and notifyPipe is replaced with simpler and more suitable eventfd. Added are threadMainWait_xxx() functions as alternative to threadMain_xxx(). They include some additional posix interface.

[henrikbrixandersen]

I see that you have removed the option for not using dynamic memory allocation. Any chance of bringing that back?

[CANopenNode]

I tried to simplify CANopen.c. But no problem, I will bring it back.

[CANopenNode]

Globals are now enabled from CANopen.c as before.

split-driver branch is now merged into master. Directory structure is rearranged as explained in #161. Other driver files are removed from master, see deviceSupport.md for details.

Old layout is still in git history. There is also old-master branch, which can be used for pull requests.

I'm closing this now, discussion of source code organization may continue in #177.

[geoffrey-vl]

any reason why the old-master branch shouldn't be called "v1" or something alike?

[CANopenNode]

You are right. I renamed it to v1.3-master.

There is also tag v1.3, but it is not the same, because v1.3-master branch already has some updates.