apache
Minor can driver improvement
Summary
- can: Remove CANIOC_XXX macro from include/nuttx/can.h
- can: Add g_ prefix to can_dlc_to_len and len_to_can_dlc
- can: Merge cd_error and rx_overflow into rx_error
Impact
can driver
Testing
internal test case
@xiaoxiang781216 could you please share your internal test? We need to confirm this modification will not break existing applications using CAN
@xiaoxiang781216 I think include/nuttx/can.h is legacy code, when I created the mcp2515 driver I needed to create a drivers/can and moved can.c to there, but I think this header file was left behind. Maybe we could merge it into include/nuttx/can/can.h
@xiaoxiang781216 I think include/nuttx/can.h is legacy code, when I created the mcp2515 driver I needed to create a drivers/can and moved can.c to there, but I think this header file was left behind. Maybe we could merge it into include/nuttx/can/can.h
include/nuttx/can/can.h contains more ioctl than include/nuttx/can.h. This is the reason why I remove ioctl from include/nuttx/can.h.
@xiaoxiang781216 I think include/nuttx/can.h is legacy code, when I created the mcp2515 driver I needed to create a drivers/can and moved can.c to there, but I think this header file was left behind. Maybe we could merge it into include/nuttx/can/can.h
include/nuttx/can/can.h contains more ioctl than include/nuttx/can.h. This is the reason why I remove ioctl from include/nuttx/can.h.
Right, but maybe it makes more sense to have only include/nuttx/can/can.h
Your mean merge include/nuttx/can.h into include/nuttx/can/can.h?
Yes!
That is not trivial. Having two can.h is needed to maintain both CAN char driver and Socket can driver. I've been looking there a while ago and didn't find an easy solution.
Your mean merge include/nuttx/can.h into include/nuttx/can/can.h?
Yes!
That is not trivial. Having two
can.his needed to maintain both CAN char driver and Socket can driver. I've been looking there a while ago and didn't find an easy solution.
Maybe we could separate it in include/nuttx/can/can.h and include/nuttx/can/socketcan.h since technically they are different
include/nuttx/can.h follow linux style(include/uapi/linux/can.h). Linux remove the support of CAN char driver, so it isn't a problem to short SocketCAN to CAN.
include/nuttx/can.hfollow linux style(include/uapi/linux/can.h). Linux remove the support of CAN char driver, so it isn't a problem to short SocketCAN to CAN.
Right, but for NuttX it is important to keep support for CAN char driver because some boards don't have enough memory to support SocketCAN.
I rather keep the Char Device CAN and SocketCAN separate, they're both fundamentally different subsystems with their own API's.
I rather keep the Char Device CAN and SocketCAN separate, they're both fundamentally different subsystems with their own API's.
Agree!
Yes, I would suggest that we write a general net driver on top of can driver, so the user just need write char driver and expose char or socketcan interface in defconfig.
That would incur a lot of overhead (Zephyr is doing it and their SocketCAN performance isn't that great due to it) It isn't hard to support both back-ends for a can driver I did it for the LPC17XX. https://github.com/apache/nuttx/blob/master/arch/arm/src/lpc17xx_40xx/lpc17_40_can.c
@xiaoxiang781216 do you agree to close it?
No, this patch series just try to fix the dup in can char driver, not the dup between can char and cat socket. So, it's still valid.