Differences
This shows you the differences between two versions of the page.
Both sides previous revisionPrevious revisionNext revision | Previous revisionNext revisionBoth sides next revision | ||
user:embedded_c_code_doesn_t_have_to_be_ugly [2020/05/11 16:36] – [3. #define vs. enum] Igor Yefmov | user:embedded_c_code_doesn_t_have_to_be_ugly [2020/05/11 17:43] – [4. Bit manipulations vs. structured data] Igor Yefmov | ||
---|---|---|---|
Line 18: | Line 18: | ||
Even if unit tests are not something that pop to mind when working on firmware (embedded) code, one must always make sure to fully understand the code before modifying it. | Even if unit tests are not something that pop to mind when working on firmware (embedded) code, one must always make sure to fully understand the code before modifying it. | ||
- | ==== 1. IDE setup ==== | + | ==== 1. Use *YOUR* favorite |
- | The default SDK provided an IDE based around [[https:// | + | The default SDK provided an IDE based around [[https:// |
* My favorite keyboard shortcuts (A mouse? It often just slows me down when I'm working with text!) | * My favorite keyboard shortcuts (A mouse? It often just slows me down when I'm working with text!) | ||
* Code browsing database | * Code browsing database | ||
+ | * Mouse-over function/ | ||
+ | * Datatype information (especially useful for '' | ||
* "Find all references" | * "Find all references" | ||
+ | * Search text in: block, file, all open files, all files for the project/ | ||
* Jump to declaration/ | * Jump to declaration/ | ||
* View call hierarchy (both " | * View call hierarchy (both " | ||
Line 95: | Line 98: | ||
==== 4. Bit manipulations vs. structured data ==== | ==== 4. Bit manipulations vs. structured data ==== | ||
+ | Working with the low-level code((which is, basically, pretty much all the embedded code)) quite often requires direct bit manipulation. And as much as every '' | ||
+ | Here's a more concrete example: working with 16/32 bit integers, where depending on the context it is either a CPU-addressable word, or a stream of bytes on the wire (be it USB or I²C). Traditionally that would result in the 1970's style code like this: | ||
+ | <code C> | ||
+ | uint8_t location[4]; | ||
+ | location[1] = (byteAddress >> 16) & 0xFF; /* MS byte */ | ||
+ | location[2] = (byteAddress >> 8) & 0xFF; | ||
+ | location[3] = byteAddress & 0xFF; /* LS byte */ | ||
+ | </ | ||
+ | |||
+ | Do you see an **obvious bug** in that code((yes, this is what was in the code! An uninitialized first byte of the '' | ||
+ | |||
+ | Here's a much more modernized version of this, which also (automagically!!!) takes care of that idiotic bug: | ||
+ | <code C> | ||
+ | uint16_t v; | ||
+ | struct{ uint8_t l, h; }; | ||
+ | uint8_t a[2]; | ||
+ | } PackedUint16; | ||
+ | typedef union PackedInt16{ | ||
+ | int16_t v; | ||
+ | struct{ uint8_t l, h; }; | ||
+ | uint8_t a[2]; | ||
+ | } PackedInt16; | ||
+ | typedef union PackedUint32{ | ||
+ | uint32_t v; | ||
+ | struct{ PackedUint16 l, h; }; | ||
+ | uint8_t a[4]; | ||
+ | } PackedUint32; | ||
+ | |||
+ | /////////////////////////////////// | ||
+ | |||
+ | PackedUint32 byteAddress = {pageAddress * glSpiPageSize}; | ||
+ | uint8_t location[4] = {}; /* uninitialized memory is evil */ | ||
+ | location[1] = byteAddress.h.l; | ||
+ | location[2] = byteAddress.l.h; | ||
+ | location[3] = byteAddress.l.l; | ||
+ | |||
+ | /* alternatively the code above can be written much more to-the-point, | ||
+ | const uint8_t * const location_arr = pageAddress.a;</ | ||
+ | |||
+ | === A much more " | ||
+ | Incoming USB requests have a standard " | ||
+ | <code C>static CyBool_t CyFxUVCApplnUSBSetupCB(uint32_t setupdat0, /* SETUP Data 0 */ | ||
+ | | ||
+ | ) { | ||
+ | CyU3PIoMatrixConfig_t io_cfg; | ||
+ | CyBool_t uvcHandleReq = CyFalse; | ||
+ | uint32_t status; | ||
+ | CyU3PReturnStatus_t apiRetStatus = CY_U3P_SUCCESS; | ||
+ | uint8_t address, data; | ||
+ | |||
+ | /* Obtain Request Type and Request */ | ||
+ | bmReqType = (uint8_t) (setupdat0 & CY_FX_USB_SETUP_REQ_TYPE_MASK ); | ||
+ | bRequest = (uint8_t) ((setupdat0 & CY_FX_USB_SETUP_REQ_MASK ) >> 8); | ||
+ | bType = (bmReqType & CY_U3P_USB_TYPE_MASK); | ||
+ | bTarget = (bmReqType & CY_U3P_USB_TARGET_MASK); | ||
+ | wValue = (uint16_t) ((setupdat0 & CY_FX_USB_SETUP_VALUE_MASK ) >> 16); | ||
+ | wIndex = (uint16_t) (setupdat1 & CY_FX_USB_SETUP_INDEX_MASK ); | ||
+ | wLength = (uint16_t) ((setupdat1 & CY_FX_USB_SETUP_LENGTH_MASK ) >> 16); | ||
+ | |||
+ | /* Handle USB vendor requests */ | ||
+ | if (bType == CY_U3P_USB_VENDOR_RQT) { | ||
+ | </ | ||
+ | The function goes on for HUNDREDS of lines of deep nesting!!!((over 600 LoC to handle the standard USB requests)). | ||
+ | |||
+ | Don't know about you, but all this bit shifting and byte extraction just screams " | ||
+ | <code C>enum UsbRequestTarget{usb_tgt_dev, | ||
+ | enum UsbRequestType{usb_rt_std, | ||
+ | typedef struct UsbSetupPacket{ | ||
+ | union{ | ||
+ | uint32_t d0; | ||
+ | struct{ | ||
+ | struct{ | ||
+ | struct{ | ||
+ | enum UsbRequestTarget bTarget : 5; | ||
+ | enum UsbRequestType bType : 2; | ||
+ | uint8_t dirIn : 1; // " | ||
+ | }; | ||
+ | uint8_t bRequest; | ||
+ | }; | ||
+ | union{ | ||
+ | PackedUint16 wValue; | ||
+ | uint16_t featureSelector; | ||
+ | }; | ||
+ | }; | ||
+ | }; | ||
+ | union{ | ||
+ | uint32_t d1; | ||
+ | struct{ | ||
+ | union{ | ||
+ | PackedUint16 wIndex; | ||
+ | struct{ | ||
+ | union{ | ||
+ | uint8_t all; | ||
+ | struct{ | ||
+ | uint8_t num : 4; | ||
+ | uint8_t res3 : 3; | ||
+ | uint8_t dirIn : 1; | ||
+ | }; | ||
+ | }; | ||
+ | uint8_t res8 : 8; | ||
+ | } ep; | ||
+ | struct Infc{ | ||
+ | uint8_t num; | ||
+ | uint8_t res; | ||
+ | } infc; | ||
+ | }; | ||
+ | PackedUint16 wLength; | ||
+ | }; | ||
+ | }; | ||
+ | } UsbSetupPacket; | ||
+ | </ | ||
+ | |||
+ | With that little prep work the same code becomes so much more readable and maintainable: | ||
+ | <code C>static CyBool_t CyFxUVCApplnUSBSetupCB(uint32_t setupdat0, uint32_t setupdat1) | ||
+ | { | ||
+ | UsbSetupPacket req = {}; | ||
+ | // the next 2 lines is all it takes to distribute the bits into proper NAMED variables | ||
+ | req.d0 = setupdat0; | ||
+ | req.d1 = setupdat1; | ||
+ | |||
+ | // dispatch to appropriate handler based on request type and return status of request handling to the USB driver | ||
+ | switch(req.bType){ | ||
+ | case usb_rt_vendor: | ||
+ | case usb_rt_std: | ||
+ | case usb_rt_class: | ||
+ | default: return CyFalse; | ||
+ | } | ||
+ | } | ||
+ | </ | ||
+ | |||
+ | Not only the code becomes that much more readable - it also opens up opportunities for compilers to do platform-specific optimizations which are otherwise just not available to them as the intent of the code is not clearly expressed with all that manual bit shifting... | ||
==== 5. Globals ==== | ==== 5. Globals ==== | ||