Hi Lars,

see my comments below
>>> - Here is a high level view of the current and suggested new API
>>> Current driver has 5 public functions:
>>> * CAN_HANDLE open(s_BOARD* (name, baudrate)
>>> * close(CAN_HANDLE)
>>> * setbaudrate(CAN_HANDLE, baudrate)
>>> * send(canmsg)
>>> * canmsg recive
>>> The new API has only one public function
>>> * driverStruct openEx(const char name).
>>> The open function does fill the driverStructure with function pointers
>>> and variable, do some basic init stuff procedure.
>>> The function does not set baud rate or start the device.
>>> driverStruct
>>> {
>>>     void*    pointer to driver internal struct. Like current CAN_HANDLE
>>>     long int supported CtrlMode. bit-field indicating what function mode
>>> the driver supports (fd, fd non-is, listen only etc.)
>>>     setBaudrate (function pointer) to a function that set main and
>>> databaudrate individually..... maybe better to set them at the same
>>> time?
>>>     send        (function pointer) same function as current API
>>>     recive      (function pointer) same function as current API
>>>     setState    (function pointer) start/stop the driver
>>>     //The functions pointer below are optional and may be set to NULL
>>>     getCanState //active, warning passive etc.
>>>     getErrorCounters //get tx/rx counter
>>>     setMode (turns on and off multiple modes at the same time)
>>> }
>>> Using the new API to init the driver one must call multiple functions
>>> to replace the current open function
>>> 1. openEx       to get the driver structure
>>> 2. setBaudrate  to set baud rate
>>> 3. setState     to start the device
>>> - Why use struck and more functions:
>>> The reason for splitting up open into multiple stages is to enable
>>> function like autobaudrate.
>>> There are multiple reason for using.
>>> 1. One can add optional functions and test for support in runtime by
>>> checking if the function pointer is NULL
>>> 2. One can create a layered structure by replacing send/receive
>>> function with one that does something with the message before it is
>>> sent or receive.
>>>      To make a software filter:
>>>      1. Get the struct from the driver
>>>      2. Make a copy of the struct and change the receive function to
>>> filter function that when call calles the driver receive function and
>>> checks the message and then optional passes it on.
>>>      Implementing CiA 302 (redundancy) can also be a lot simpler to
>>> create a layer between the driver and the normal caneopen stack
>> Iooks nice, this driverStruct may be seen like the big co_data struct of
>> canfestival, an object containing a complete instance of the driver.
>> I like the similarity of those two structs.
>> In the same spirit, i would prefer to see the getCanState and
>> getErrorCounter as callback functions.
>> Something like canStateChange() and errorCounterChange() called by the
>> driver when update occurs.
>> If the driver implement this, it provides default callbacks that does
>> nothing, if it does not implement this functions pointers are set to NULL.
>> Then the user is free to register his own callbacks if needed.
> That is a god idea, i can add a "checkForChange" function that checks
> for changes and calls the callback on changes.

I was thinking of avoiding polling, providing an asynchronous API.
May be it could work like this, at least for socketcan:

1- configure socketcan to allow the reception of error frames

2- in SocketFD_receive() when a frame is received check canid to see if 
its an error frame or a data frame

3- if data frame return like you already do

4- If it is an error frame then decode the error or status change (see 
     Then call the callback and after don't return from the function but 
loop to can receive again

May be we can also provide a polling API by updating a status and error 
variable that can be read via access functions like getState()
This way user can choose between asynchronous or polling API.

>> May be we need a more general callback for errors, i think of all errors
>> reported by socketcan, each time the driver receives an error frame it
>> calls the callback with the error code as parameter.
> I see that QT uses something similar to socketCan error frame.
> http://doc.qt.io/qt-5/qcanbusframe.html
> Can we just do the same?
> Add a variable to the Message that says it is a error frame and return
> if from canReceive_driver like a normal can message.
This is what already exists in socketcan, bit29 in canid indicates it is 
not a data frame but an error frame.
The type of error /  event is coded in the datas with DLC=8
Can we take this as a general can errors abstraction and use it also for 
other drivers ?

Instead of doing what i suggested for asynchronous call of error 
callbacks, we could return the error frame to user and let him filter 
and decide what to do.
But this will require a change to canfestival to be able to provide this 
error information to CF user.


Fran├žois Beaulier

Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Canfestival-devel mailing list

Reply via email to