Hi Lars,
Low level CAN error reporting is in fact not really a CanOpen issue.
So this error reporting is just a nice feature that the user can
optionally use if needed.
It is not for canfestival itself, the stack will never call error
reporting functions.
So as you said, it is free software, make a driver with the
functionalities you need, and others may extend it if they need to.
Post here a first release when it's ready and we'll see.
Regards
François Beaulier
>>>>> - 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
>> /usr/include/linux/can/error.h)
>> 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.
> Thanks for explaining.
> Well that will work in the socketcan world and in the AVR (micro
> controller with 2-4k ram) with some rework. Other that that i dont
> know :(
> The one problem i see on socketcan that AFIK there is no simple way we
> can find out if the kernel driver for the device we use will output
> the error frames we are using, or any error frames for that matter.
> If we don't know, how can we make functionality that depend on it?
>
> By pooling the driver we will get a error if it is not supporter so we
> can check that when we init the driver. The constant checking and
> calling the callback function can be done from the update-thread so it
> will be invisible to the user. On more resource restrained platforms
> like micro controllers it will be a function you can call if you
> want/need it.
>
>>>> 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 filte
>> and decide what to do.
>> But this will require a change to canfestival to be able to provide this
>> error information to CF user.
> Been thinking about this and i do not know what to say., but i
> appreciate you taking the time to think about it.
> I love the idea of a general error reporting system and using can
> messages looks great, but how should the user know what messages the
> can-festival driver makes?
> If the users or can-festival do not know what messages the driver
> makes he/she can not make sw that depend on it.
> I also have very limited knowledge of what the user needs when it
> comes to error reporting as I only use rx/tx error counter and the
> state (active,warning,passive,off) and that is all that i need.
>
> As i have so limited knowledge and need more time to make up my mind i
> will not be including the following:
> reading and usage of socketcan error frames (AFIK i dont know how to
> find out if the kernel driver will be sending the message i want)
> a general error reporting system using error frames inside canFestival
> (limited knowledge of what the user wants)
>
> As this is open source feel free to add it yourself.
> I will comment to the best of my ability on any attempt, but i will
> not make it myself...at this time.
>
>> Regards
>>
>> François Beaulier
> Regards
> Lars E. Susaas
>
------------------------------------------------------------------------------
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
Canfestival-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/canfestival-devel
