• State
  • Locked Locked
  • Replies 3 replies
  • Subscribers 82 subscribers
  • Views 125 views
  • Users 0 members are here
Support feedback
Options
Options
Similar topics

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

TMS320F28388D: Driverlib - Inconsistent Style

Wil P.
Expert 1640 points
Part Number: TMS320F28388D

Just want to provide some feedback.

I understand that TI is leaning into programming and configuration through driverlib, but IMO, the code style is inconsistent and often hard to read.

Here are a few examples found in ethernet.h/c in F2838x driverlib_cm:

  • White space
    • There is a anywhere between 1-4 lines between code entities (such as block documentation, struct/enum definitions, etc)
    • The code (mostly) uses spaces for indentation, so it is independent of tab-width definitions (good), but the indentation is inconsistent even in the same entity (such as a struct definition).
    • Spacing before or after the asterik to declare pointer.
  • Bracket placement
    • typedef enum/struct definitions sometimes have a new line before the opening bracket and sometimes not, almost at random. Sometimes there are two line breaks
  • Variable / function Naming
    • Mostly its <device>_xxx, but the xxx part is the wild west.  Sometimes words are camel cased, sometimes they are separated by underscore

Here is an unedited snippet that demonstrates a few examples.  For anyone adversely affected by this, I apologize.

Fullscreen
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
//*****************************************************************************
//
//! The structure for Rx Queue Configuration
//! Used by the driver in Rx Queue Configuration
//
//*****************************************************************************
typedef struct {
uint32_t reserved_0:4;
uint32_t PTPQ:1;
uint32_t reserved_1:7;
uint32_t UPQ:1;
uint32_t reserved_2:3;
uint32_t MCBCQ:1;
uint32_t reserved_3:3;
uint32_t MCBCQEN:1;
uint32_t reserved_4:1;
uint32_t TPQC:1;
uint32_t reserved_5:9;
} Ethernet_RxQControl;
typedef enum
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

  • 0
    TI__Mastermind 34635 points

    Hi, 

    Thanks for the feedback.  Is this ovservation specific to ethernet driver  ? 

    Will look into ways to improve the coding style going forward.  I think white space and bracket placement can be certainly taken up. 

    For the variable/function naming, if it changed for the existing drivers , it will cause compatibility issues with the older version.  For any new code development, we will consider this.

    Best Regards

    Siddharth

  • 0 in reply to Siddharth Deshpande
    Expert 1640 points

    It is not specific to the ethernet driver. My overall point is that it is not similar across the files and certainly not across devices.

    I am not sure about the rules regarding compatibility across devices, but this is one of the reasons that I (used to) avoid driverlib. When I move to different MCUs (or try to maintain a codebase across different platforms), its a mess of nested conditionals. Again, here is just one example, XBAR_setInputPin.

    On the F2838x:

    static inline void
    XBAR_setInputPin(uint32_t base, XBAR_InputNum input, uint16_t pin)

    On the F2837x:

    static inline void
    XBAR_setInputPin(XBAR_InputNum input, uint16_t pin)

    The bit-fields tend to be a bit more friendly to legacy support. Anyway, I feel like I am just whining now. Love you guys.

    EDIT: Another example: pin_map.h

    On the F2838x: GPIO_36_CANA_RX

    On the F2837x: GPIO_36_CANRXA

  • 0 in reply to Wil P.
    TI__Mastermind 34635 points

    Hi, 

    Will improve on this going forward.  

    Best Regards

    Siddharth