Differences

This shows you the differences between two versions of the page.

--- user:embedded_c_code_doesn_t_have_to_be_ugly [2020/05/11 17:18] – [4. Bit manipulations vs. structured data] 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 138: / Line 138: @@
 const uint8_t * const location_arr = pageAddress.a;</code>
+=== A much more "complicated" example ===
+Incoming USB requests have a standard "setup" header which results in old-style code looking something like the following((there's a separate discussion on the use of globals later on)):
+<code C>static CyBool_t CyFxUVCApplnUSBSetupCB(uint32_t setupdat0, /* SETUP Data 0 */
+                                       uint32_t setupdat1 /* SETUP Data 1 */
+) {
+    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) {
+</code>
+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 "fruitful field for errors!" to me. Consider having abstracted all this ugliness into a structure like the following:
+<code C>enum UsbRequestTarget{usb_tgt_dev, usb_tgt_ifc, usb_tgt_ep, usb_tgt_other};
+enum UsbRequestType{usb_rt_std, usb_rt_class, usb_rt_vendor, usb_rt_reserved};
+typedef struct UsbSetupPacket{
+    union{
+        uint32_t d0;
+        struct{
+            struct{
+                struct{
+                    enum UsbRequestTarget bTarget : 5;
+                    enum UsbRequestType bType : 2;
+                    uint8_t dirIn : 1; // "IN" is the direction towards USB host (reading data)
+                };
+                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;
+</code>
+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: return handleVendorRqt(&req);
+        case usb_rt_std:    return handleUsbStdRqt(&req);
+        case usb_rt_class:  return handleUvcRqt(&req);
+        default: return CyFalse;
+    }
+}
+</code>
+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 ====

SUB2r

User Tools

Differences