Hi,

Sorry for the long delay.
The code is now checked inn at:
https://bitbucket.org/nimrof/canfestival-3-asc/branch/canfd

Some notes:
* canFD does not work. I failed to get the device into canFD mode, but
still working on it.
* Only the example CANOpenShellMasterOD is portet to the new API so a
complete build will fail
* Sorry windows plp, but only unix driver is portet
* socketcan driver is fully portet, but watch out for bugs :)
* the virtual driver has been semi portet. That the minimal job that
is needed to make the current driver API work
* Its not that well testet so watch out for bugs

On Mon, Apr 24, 2017 at 10:09 AM,
<canfestival-devel@lists.sourceforge.net> wrote:
> 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

------------------------------------------------------------------------------
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

Reply via email to