Hi Lars,
Sorry for the delay, i really appreciate your effort to improve
canfestival and i don't want to leave you alone on this !
I strongly encourage CF users to give feedback and show their interest.
> The current driver API need to be extended to be able to do the following:
> * Enable can-FD mode
> * Check if driver support can-FD
> * Set data-baudtrate
> * Receive/send normal and can-fd messages
> * switch from can-fd bosh and can-fd iso (possible on some hw)
>
> So when one is to change the drivers anyway could this be the time to
> add more functionality to the driver API?
> * hw/kernel can filter support
> * get can state (active,warning,passive, off)
> * start/stop/reset driver
> * get rx,tx error counter
> * listen only mode. (For autobaudrate usage)
>
> I think so.
I agree
> But if it ends up with heavy rewriting of all the drivers then no.
> As a test i have written a (almost) complete socketcan driver using
> the suggested new API.
> >From what i have learned to takes some work to rewrite drivers to
> suggested new API so the current and new API should be able to live
> along each other.
Ok but i think in most cases the app use only one driver so it is a
matter of just knowing whether a new driver with new API is available
for my CAN hardware.
> - 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.
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.
> - How to support new and current API at the same time?
>
> As far as i can see it is during startup and shutdown things need to
> take different directions
>
> When using dynamic loading (.so/.dll)
> If openEx is found use the new way of initing the driver. If it is
> not found init the driver to current way.
> This can be done in runtime
> On close check for openEx again and close using different ways
> When static linking
> As i have not found a way to detect if a function exist i compiler
> time it must be solved using preprocessor
>
> When init is done, make a driver structure and copy send&receive
> function into it.
> >From now on one can use a common interface
As i said, i don't think driver API detection is very important.
But may be i'm wrong.
> Thats it and thanks for reading.
> I will attach can_socketFD, my mostly complete implementation. Missing
> start/stop, setMode and real testing....so it does have bugs
> can_driver.h includes new structures and enums needed by the new API
>
> So thats it and thanks for reading it all.
>
> Do please comment on my suggestions and ask questions
>
>
Some ideas:
- be sure to be able to have several instances of the same driver in a
single process
- in case of USB to CAN adapters be resistant to live unplug / replug
I'm ready to test your new socketcan driver when you think it's done.
Regards
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
Canfestival-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/canfestival-devel
