Review comments.

MiniDSP.cpp

@@ -14,30 +14,13 @@
 
 #include "MiniDSP.h"
 
-namespace {
+void MiniDSP::ParseHIDData(USBHID *hid __attribute__((unused)), bool is_rpt_id __attribute__((unused)), uint8_t len, uint8_t *buf) {
-uint8_t RequestStatusOutputCommand[] = {0x05, 0xFF, 0xDA, 0x02};
-uint8_t StatusInputCommand[]{0x05, 0xFF, 0xDA};
 
-// Returns first byte of the sum of given bytes.
+        constexpr uint8_t StatusInputCommand[]{0x05, 0xFF, 0xDA};
-uint8_t Checksum(const uint8_t *data, uint16_t nbytes) {
-  uint64_t sum = 0;
-  for (uint16_t i = 0; i < nbytes; i++) {
-    sum += data[i];
-  }
-
-  return sum & 0xFF;
-}
-} // namespace
-
-void MiniDSP::ParseHIDData(USBHID *hid __attribute__((unused)),
-                           bool is_rpt_id __attribute__((unused)), uint8_t len,
-                           uint8_t *buf) {
 
         // Only care about valid data for the MiniDSP 2x4HD.
-  if (HIDUniversal::VID != MINIDSP_VID || HIDUniversal::PID != MINIDSP_PID ||
+        if (HIDUniversal::VID != MINIDSP_VID || HIDUniversal::PID != MINIDSP_PID || len <= 4 || buf == nullptr)
-      len <= 2 || buf == nullptr) {
                 return;
-  }
 
         // Check if this is a status update.
         // First byte is the length, we ignore that for now.
@@ -48,46 +31,46 @@ void MiniDSP::ParseHIDData(USBHID *hid __attribute__((unused)),
                 const auto newVolume = buf[sizeof(StatusInputCommand) + 1];
                 const auto newIsMuted = (bool)buf[sizeof(StatusInputCommand) + 2];
 
-    const auto volumeUpdated = newVolume != volume;
-    const auto isMutedUpdated = newIsMuted != isMuted;
-
                 // Update status.
                 volume = newVolume;
-    isMuted = newIsMuted;
+                muted = newIsMuted;
 
                 // Call callbacks.
-    if (volumeChangeCallback != nullptr && volumeUpdated) {
+                if (pFuncOnVolumeChange != nullptr && newVolume != volume)
-      volumeChangeCallback(volume);
+                        pFuncOnVolumeChange(volume);
-    }
 
-    if (mutedChangeCallback != nullptr && isMutedUpdated) {
+                if (pFuncOnMutedChange != nullptr && newIsMuted != muted)
-      mutedChangeCallback(isMuted);
+                        pFuncOnMutedChange(muted);
-    }
         }
 };
 
 uint8_t MiniDSP::OnInitSuccessful() {
         // Verify we're actually connected to the MiniDSP 2x4HD.
-  if (HIDUniversal::VID != MINIDSP_VID || HIDUniversal::PID != MINIDSP_PID) {
+        if (HIDUniversal::VID != MINIDSP_VID || HIDUniversal::PID != MINIDSP_PID)
                 return 0;
-  }
 
         // Request current status so we can initialize the values.
         RequestStatus();
 
-  if (onInitCallback != nullptr) {
+        if (pFuncOnInit != nullptr)
-    onInitCallback();
+                pFuncOnInit();
-  }
 
         return 0;
 };
 
-void MiniDSP::SendCommand(uint8_t *command, uint16_t command_length) const {
+uint8_t MiniDSP::Checksum(const uint8_t *data, uint8_t data_length) const {
-  // Only send command if we're actually connected to the MiniDSP 2x4HD.
+        uint16_t sum = 0;
-  if (HIDUniversal::VID != MINIDSP_VID || HIDUniversal::PID != MINIDSP_PID) {
+        for (uint8_t i = 0; i < data_length; i++)
-    return;
+                sum += data[i];
+
+        return sum & 0xFF;
 }
 
+void MiniDSP::SendCommand(uint8_t *command, uint8_t command_length) const {
+        // Sanity check on command length.
+        if (command_length > 63)
+                return;
+
         // Message is padded to 64 bytes with 0xFF and is of format:
         // [ length (command + checksum byte) ] [ command ] [ checksum ] [ OxFF... ]
 
@@ -108,10 +91,11 @@ void MiniDSP::SendCommand(uint8_t *command, uint16_t command_length) const {
         // Pad the rest.
         memset(&buf[checksumOffset + 1], 0xFF, sizeof(buf) - checksumOffset - 1);
 
-  pUsb->outTransfer(bAddress, epInfo[epInterruptOutIndex].epAddr, sizeof(buf),
+        pUsb->outTransfer(bAddress, epInfo[epInterruptOutIndex].epAddr, sizeof(buf), buf);
-                    buf);
 }
 
 void MiniDSP::RequestStatus() const {
+        uint8_t RequestStatusOutputCommand[] = {0x05, 0xFF, 0xDA, 0x02};
+
         SendCommand(RequestStatusOutputCommand, sizeof(RequestStatusOutputCommand));
 }

MiniDSP.h

@@ -14,7 +14,6 @@
 
 #pragma once
 
-#include "controllerEnums.h"
 #include "hiduniversal.h"
 
 #define MINIDSP_VID 0x2752 // MiniDSP
@@ -43,49 +42,61 @@ public:
          * @return Returns true if it is connected.
          */
         bool connected() {
-    return HIDUniversal::isReady() && HIDUniversal::VID == MINIDSP_VID &&
+                return HIDUniversal::isReady() && HIDUniversal::VID == MINIDSP_VID && HIDUniversal::PID == MINIDSP_PID;
-           HIDUniversal::PID == MINIDSP_PID;
         };
 
         /**
-   * Used to call your own function when the device is successfully initialized.
+         * Used to call your own function when the device is successfully
-   * @param func Function to call.
+         * initialized.
+         * @param funcOnInit Function to call.
          */
-  void SetOnInitCallback(void (*func)(void)) { onInitCallback = func; };
+        void attachOnInit(void (*funcOnInit)(void)) { pFuncOnInit = funcOnInit; };
 
         /**
          * Used to call your own function when the volume has changed.
-   * The volume is passed as an unsigned integer that represents twice the -dB
+         * The volume is passed as an unsigned integer that represents twice the
-   * value. Example: 19 represents -9.5dB.
+         * -dB value. Example: 19 represents -9.5dB.
-   * @param func Function to call.
+         * @param funcOnVolumeChange Function to call.
          */
-  void SetVolumeChangeCallback(void (*func)(uint8_t)) {
+        void attachOnVolumeChange(void (*funcOnVolumeChange)(uint8_t)) {
-    volumeChangeCallback = func;
+                pFuncOnVolumeChange = funcOnVolumeChange;
         }
 
         /**
          * Used to call your own function when the muted status has changed.
-   * The muted status is passed as a boolean. True means muted, false means
+         * The muted status is passed as a boolean. True means muted, false
-   * unmuted.
+         * means unmuted.
-   * @param func Function to call.
+         * @param funcOnMutedChange Function to call.
          */
-  void SetMutedChangeCallback(void (*func)(bool)) {
+        void attachOnMutedChange(void (*funcOnMutedChange)(bool)) {
-    mutedChangeCallback = func;
+                pFuncOnMutedChange = funcOnMutedChange;
         }
 
         /**
          * Retrieve the current volume of the MiniDSP.
-   * The volume is passed as an unsigned integer that represents twice the -dB
+         * The volume is passed as an unsigned integer that represents twice the
-   * value. Example: 19 represents -9.5dB.
+         * -dB value. Example: 19 represents -9.5dB.
          * @return Current volume.
          */
-  int GetVolume() const { return volume; }
+        int getVolume() const {
+                return volume;
+        }
+
+        /**
+         * Retrieve the current volume of the MiniDSP in -dB.
+         * @return Current volume.
+         */
+        float getVolumeDB() const {
+                return volume / -2.0;
+        }
 
         /**
          * Retrieve the current muted status of the MiniDSP
          * @return `true` if the device is muted, `false` otherwise.
          */
-  bool IsMuted() const { return isMuted; }
+        bool isMuted() const {
+                return muted;
+        }
 
       protected:
         /** @name HIDUniversal implementation */
@@ -101,7 +112,8 @@ protected:
         /**
          * Called when a device is successfully initialized.
          * Use attachOnInit(void (*funcOnInit)(void)) to call your own function.
-   * This is useful for instance if you want to set the LEDs in a specific way.
+         * This is useful for instance if you want to set the LEDs in a specific
+         * way.
          */
         uint8_t OnInitSuccessful();
         /**@}*/
@@ -111,7 +123,8 @@ protected:
          * Used by the USB core to check what this driver support.
          * @param  vid The device's VID.
          * @param  pid The device's PID.
-   * @return     Returns true if the device's VID and PID matches this driver.
+         * @return     Returns true if the device's VID and PID matches this
+         * driver.
          */
         virtual bool VIDPIDOK(uint16_t vid, uint16_t pid) {
                 return vid == MINIDSP_VID && pid == MINIDSP_PID;
@@ -120,38 +133,47 @@ protected:
 
       private:
         /**
-   * Send the "Request status" command to the MiniDSP. The response includes the
+         * Calculate checksum for given buffer. 
-   * current volume and the muted status.
+         * Checksum is given by summing up all bytes in `data` and returning the first byte.
+         * @param data Buffer to calculate checksum for.
+         * @param data_length Length of the buffer.
          */
-  void RequestStatus() const;
+        uint8_t Checksum(const uint8_t *data, uint8_t data_length) const;
 
         /**
-   * Send the given MiniDSP command. This function will create a buffer with the
+         * Send the "Request status" command to the MiniDSP. The response
-   * expected header and checksum and send it to the MiniDSP. Responses will
+         * includes the current volume and the muted status.
-   * come in throug `ParseHIDData`.
+         */
+        void
+        RequestStatus() const;
+
+        /**
+         * Send the given MiniDSP command. This function will create a buffer
+         * with the expected header and checksum and send it to the MiniDSP.
+         * Responses will come in throug `ParseHIDData`.
          * @param command Buffer of the command to send.
          * @param command_length Length of the buffer.
          */
-  void SendCommand(uint8_t *command, uint16_t command_length) const;
+        void SendCommand(uint8_t *command, uint8_t command_length) const;
 
         // Callbacks
 
         // Pointer to function called in onInit().
-  void (*onInitCallback)(void) = nullptr;
+        void (*pFuncOnInit)(void) = nullptr;
 
         // Pointer to function called when volume changes.
-  void (*volumeChangeCallback)(uint8_t) = nullptr;
+        void (*pFuncOnVolumeChange)(uint8_t) = nullptr;
 
         // Pointer to function called when muted status changes.
-  void (*mutedChangeCallback)(bool) = nullptr;
+        void (*pFuncOnMutedChange)(bool) = nullptr;
 
         // -----------------------------------------------------------------------------
 
-  // MiniDSP state. Currently only volume and muted status are implemented, but
+        // MiniDSP state. Currently only volume and muted status are
-  // others can be added easily if needed.
+        // implemented, but others can be added easily if needed.
 
         // The volume is stored as an unsigned integer that represents twice the
         // -dB value. Example: 19 represents -9.5dB.
         uint8_t volume = 0;
-  bool isMuted = false;
+        bool muted = false;
 };

examples/MiniDSP/MiniDSP.ino

@@ -1,7 +1,5 @@
 /*
- Example sketch for the Playstation Buzz library - developed by Kristian Lauszus
+ Example sketch for the MiniDSP 2x4HD library - developed by Dennis Frett
- For more information visit my blog: http://blog.tkjelectronics.dk/ or
- send me an e-mail:  kristianl@tkjelectronics.com
  */
 
 #include <MiniDSP.h>
@@ -15,7 +13,9 @@
 USB Usb;
 MiniDSP MiniDSP(&Usb);
 
-void OnMiniDSPConnected() { Serial.println("MiniDSP connected"); }
+void OnMiniDSPConnected() {
+        Serial.println("MiniDSP connected");
+}
 
 void OnVolumeChange(uint8_t volume) {
         Serial.println("Volume is: " + String(volume));
@@ -40,9 +40,11 @@ void setup() {
         Serial.println(F("\r\nMiniDSP 2x4HD Library Started"));
 
         // Register callbacks.
-  MiniDSP.SetOnInitCallback(&OnMiniDSPConnected);
+        MiniDSP.attachOnInit(&OnMiniDSPConnected);
-  MiniDSP.SetVolumeChangeCallback(&OnVolumeChange);
+        MiniDSP.attachOnVolumeChange(&OnVolumeChange);
-  MiniDSP.SetMutedChangeCallback(&OnMutedChange);
+        MiniDSP.attachOnMutedChange(&OnMutedChange);
 }
 
-void loop() { Usb.Task(); }
+void loop() {
+        Usb.Task();
+}