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 18:23] – [5. Globals] 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 ==== | ||
+ | Having global variables is absolutely the major contributor to entangling unrelated blocks of code and making sure no human mind can fully comprehend your text without first truly understanding **all of it**. "A variable is declared in one file, set in another, accessed in yet another... what fun it is to trace its lifetime!" | ||
+ | |||
+ | There are a few kinds of globals, IMO: | ||
+ | === " | ||
+ | These are the ones declared at file level and mentioned with an '' | ||
+ | |||
+ | See if some of those globals can be made file-scoped by grouping related functions together((naturally all the functions that access the same set of globals are very likely to be related to each other in functionality)). Some '' | ||
+ | |||
+ | === Function-level globals === | ||
+ | A relic of K&R style '' | ||
+ | |||
+ | These same compilers are capable of making quite intelligent choices on how to translate your code into zeroes and ones, so that you should truly be not afraid of expressing your functionality in the language of abstraction rather than writing in '' | ||
+ | |||
+ | Stack allocation is extremely cheap, even on the most low-end down-to-earth hardware, so concerning yourself with the cost of that operation by grouping all-the-possible-function-variables together at the top should be reserved only for the most critical code((in which case it is very likely that such code should be written directly in Assembler by a highly-skilled engineer who is also extremely familiar with the target platform)). | ||
+ | |||
+ | === Cross-function globals === | ||
+ | Oh, these are absolutely the worst as they have a tendency on sneaking up on you at the least expected time and place! They get declared as globals and then are only set in one place while being accessed in plenty of other places. Especially dangerous ones cross thread boundaries, making for extra-nasty race condition type bugs. | ||
+ | |||
+ | Even if one doesn' | ||
==== 6. Code blocks' | ==== 6. Code blocks' |