WiimoteEmu: Cleanup, kill some magic numbers, and unbreak wiimote speaker pan setting.

Source/Core/Core/HW/WiimoteCommon/WiimoteHid.h

@@ -22,6 +22,27 @@ struct hid_packet
   u8 data[0];
 };
 
+template <typename T>
+struct TypedHidPacket
+{
+  u8 param : 4;
+  u8 type : 4;
+
+  // TODO: auto set this from the T-type:
+  u8 report_id;
+
+  T data;
+
+  static_assert(std::is_pod<T>::value);
+
+  const u8* GetData() const { return reinterpret_cast<const u8*>(this); }
+  u32 GetSize() const
+  {
+    static_assert(sizeof(*this) == sizeof(T) + 2);
+    return sizeof(*this);
+  }
+};
+
 constexpr u8 HID_TYPE_HANDSHAKE = 0;
 constexpr u8 HID_TYPE_SET_REPORT = 5;
 constexpr u8 HID_TYPE_DATA = 0xA;

Source/Core/Core/HW/WiimoteCommon/WiimoteReport.h

@@ -32,6 +32,10 @@ enum ReportType
   RT_STATUS_REPORT = 0x20,
   RT_READ_DATA_REPLY = 0x21,
   RT_ACK_DATA = 0x22,
+
+  // Not a real value on the wiimote, just a state to disable reports:
+  RT_REPORT_DISABLED = 0x00,
+
   RT_REPORT_CORE = 0x30,
   RT_REPORT_CORE_ACCEL = 0x31,
   RT_REPORT_CORE_EXT8 = 0x32,
@@ -40,7 +44,7 @@ enum ReportType
   RT_REPORT_CORE_ACCEL_EXT16 = 0x35,
   RT_REPORT_CORE_IR10_EXT9 = 0x36,
   RT_REPORT_CORE_ACCEL_IR10_EXT6 = 0x37,
-  RT_REPORT_EXT21 = 0x3d,  // never used?
+  RT_REPORT_EXT21 = 0x3d,
   RT_REPORT_INTERLEAVE1 = 0x3e,
   RT_REPORT_INTERLEAVE2 = 0x3f
 };

Source/Core/Core/HW/WiimoteEmu/EmuSubroutines.cpp

@@ -39,6 +39,21 @@ namespace WiimoteEmu
 {
 void Wiimote::ReportMode(const wm_report_mode* const dr)
 {
+  if (dr->mode < RT_REPORT_CORE || dr->mode > RT_REPORT_INTERLEAVE2 ||
+           (dr->mode > RT_REPORT_CORE_ACCEL_IR10_EXT6 && dr->mode < RT_REPORT_EXT21))
+  {
+    // A real wiimote ignores the entire message if the mode is invalid.
+    WARN_LOG(WIIMOTE, "Game requested invalid report mode: 0x%02x", dr->mode);
+    return;
+  }
+  else if (dr->mode > RT_REPORT_CORE_ACCEL_IR10_EXT6)
+  {
+    PanicAlert("Wiimote: Unsupported Reporting mode: 0x%02x", dr->mode);
+  }
+
+  // TODO: A real wiimote sends a report immediately.
+  // even on REPORT_CORE and continuous off when the buttons haven't changed.
+
   // INFO_LOG(WIIMOTE, "Set data report mode");
   // DEBUG_LOG(WIIMOTE, "  Rumble: %x", dr->rumble);
   // DEBUG_LOG(WIIMOTE, "  Continuous: %x", dr->continuous);
@@ -49,15 +64,6 @@ void Wiimote::ReportMode(const wm_report_mode* const dr)
   // m_reporting_auto = dr->all_the_time;
   m_reporting_auto = dr->continuous;
   m_reporting_mode = dr->mode;
-  // m_reporting_channel = _channelID;	// this is set in every Interrupt/Control Channel now
-
-  // reset IR camera
-  // memset(m_reg_ir, 0, sizeof(*m_reg_ir));  //ugly hack
-
-  if (dr->mode > 0x37)
-    PanicAlert("Wiimote: Unsupported Reporting mode.");
-  else if (dr->mode < RT_REPORT_CORE)
-    PanicAlert("Wiimote: Reporting mode < 0x30.");
 }
 
 /* Here we process the Output Reports that the Wii sends. Our response will be
@@ -100,6 +106,7 @@ void Wiimote::HidOutputReport(const wm_report* const sr, const bool send_ack)
 
   case RT_IR_PIXEL_CLOCK:  // 0x13
     // INFO_LOG(WIIMOTE, "WM IR Clock: 0x%02x", sr->data[0]);
+    // Camera data is currently always updated. Ignoring pixel clock status.
     // m_ir_clock = sr->enable;
     if (false == sr->ack)
       return;
@@ -135,7 +142,7 @@ void Wiimote::HidOutputReport(const wm_report* const sr, const bool send_ack)
       auto sd = reinterpret_cast<const wm_speaker_data*>(sr->data);
       if (sd->length > 20)
         PanicAlert("EmuWiimote: bad speaker data length!");
-      m_i2c_bus.BusWrite(0x51, 0x00, sd->length, sd->data);
+      SpeakerData(sd->data, sd->length);
     }
     return;  // no ack
     break;
@@ -147,6 +154,7 @@ void Wiimote::HidOutputReport(const wm_report* const sr, const bool send_ack)
     break;
 
   case RT_IR_LOGIC:  // 0x1a
+    // Camera data is currently always updated. Just saving this for status requests.
     m_status.ir = sr->enable;
     if (false == sr->ack)
       return;
@@ -168,18 +176,19 @@ void Wiimote::HidOutputReport(const wm_report* const sr, const bool send_ack)
    The last byte is the success code 00. */
 void Wiimote::SendAck(u8 report_id, u8 error_code)
 {
-  u8 data[6];
+  TypedHidPacket<wm_acknowledge> rpt;
+  rpt.type = HID_TYPE_DATA;
+  rpt.param = HID_PARAM_INPUT;
+  rpt.report_id = RT_ACK_DATA;
 
-  data[0] = 0xA1;
+  auto ack = &rpt.data;
-  data[1] = RT_ACK_DATA;
-
-  wm_acknowledge* ack = reinterpret_cast<wm_acknowledge*>(data + 2);
 
   ack->buttons = m_status.buttons;
   ack->reportID = report_id;
   ack->errorID = error_code;
 
-  Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, data, sizeof(data));
+  Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, rpt.GetData(),
+                                         rpt.GetSize());
 }
 
 void Wiimote::HandleExtensionSwap()
@@ -218,15 +227,17 @@ void Wiimote::RequestStatus(const wm_request_status* const rs)
   m_status.battery_low = m_status.battery < 0x33;
 
   // set up report
-  u8 data[8];
+  TypedHidPacket<wm_status_report> rpt;
-  data[0] = 0xA1;
+  rpt.type = HID_TYPE_DATA;
-  data[1] = RT_STATUS_REPORT;
+  rpt.param = HID_PARAM_INPUT;
+  rpt.report_id = RT_STATUS_REPORT;
 
   // status values
-  *reinterpret_cast<wm_status_report*>(data + 2) = m_status;
+  rpt.data = m_status;
 
   // send report
-  Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, data, sizeof(data));
+  Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, rpt.GetData(),
+                                         rpt.GetSize());
 }
 
 /* Write data to Wiimote and Extensions registers. */
@@ -337,11 +348,12 @@ bool Wiimote::ProcessReadDataRequest()
     return false;
   }
 
-  u8 data[23] = {};
+  TypedHidPacket<wm_read_data_reply> rpt;
-  data[0] = 0xA1;
+  rpt.type = HID_TYPE_DATA;
-  data[1] = RT_READ_DATA_REPLY;
+  rpt.param = HID_PARAM_INPUT;
+  rpt.report_id = RT_READ_DATA_REPLY;
 
-  wm_read_data_reply* const reply = reinterpret_cast<wm_read_data_reply*>(data + 2);
+  auto reply = &rpt.data;
   reply->buttons = m_status.buttons;
   reply->address = Common::swap16(m_read_request.address);
 
@@ -417,7 +429,8 @@ bool Wiimote::ProcessReadDataRequest()
   m_read_request.size -= bytes_to_read;
 
   // Send the data
-  Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, data, sizeof(data));
+  Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, rpt.GetData(),
+                                         rpt.GetSize());
 
   return true;
 }

Source/Core/Core/HW/WiimoteEmu/Speaker.cpp

@@ -68,11 +68,11 @@ void stopdamnwav()
 }
 #endif
 
-void Wiimote::SpeakerLogic::SpeakerData(const u8* data, int length)
+void Wiimote::SpeakerData(const u8* data, int length)
 {
   if (!SConfig::GetInstance().m_WiimoteEnableSpeaker)
     return;
-  if (reg_data.volume == 0 || reg_data.sample_rate == 0 ||
+  if (m_speaker_logic.reg_data.volume == 0 || m_speaker_logic.reg_data.sample_rate == 0 ||
       length == 0)
     return;
 
@@ -82,7 +82,7 @@ void Wiimote::SpeakerLogic::SpeakerData(const u8* data, int length)
   unsigned int sample_rate_dividend, sample_length;
   u8 volume_divisor;
 
-  if (reg_data.format == 0x40)
+  if (m_speaker_logic.reg_data.format == SpeakerLogic::DATA_FORMAT_PCM)
   {
     // 8 bit PCM
     for (int i = 0; i < length; ++i)
@@ -95,13 +95,14 @@ void Wiimote::SpeakerLogic::SpeakerData(const u8* data, int length)
     volume_divisor = 0xff;
     sample_length = (unsigned int)length;
   }
-  else if (reg_data.format == 0x00)
+  else if (m_speaker_logic.reg_data.format == SpeakerLogic::DATA_FORMAT_ADPCM)
   {
     // 4 bit Yamaha ADPCM (same as dreamcast)
     for (int i = 0; i < length; ++i)
     {
-      samples[i * 2] = adpcm_yamaha_expand_nibble(adpcm_state, (data[i] >> 4) & 0xf);
+      samples[i * 2] =
-      samples[i * 2 + 1] = adpcm_yamaha_expand_nibble(adpcm_state, data[i] & 0xf);
+          adpcm_yamaha_expand_nibble(m_speaker_logic.adpcm_state, (data[i] >> 4) & 0xf);
+      samples[i * 2 + 1] = adpcm_yamaha_expand_nibble(m_speaker_logic.adpcm_state, data[i] & 0xf);
     }
 
     // Following details from http://wiibrew.org/wiki/Wiimote#Speaker
@@ -114,16 +115,16 @@ void Wiimote::SpeakerLogic::SpeakerData(const u8* data, int length)
   }
   else
   {
-    ERROR_LOG(IOS_WIIMOTE, "Unknown speaker format %x", reg_data.format);
+    ERROR_LOG(IOS_WIIMOTE, "Unknown speaker format %x", m_speaker_logic.reg_data.format);
     return;
   }
 
   // Speaker Pan
   // TODO: fix
-  unsigned int vol = 0;//(unsigned int)(m_options->numeric_settings[0]->GetValue() * 100);
+  unsigned int vol = (unsigned int)(m_options->numeric_settings[0]->GetValue() * 100);
 
-  unsigned int sample_rate = sample_rate_dividend / reg_data.sample_rate;
+  unsigned int sample_rate = sample_rate_dividend / m_speaker_logic.reg_data.sample_rate;
-  float speaker_volume_ratio = (float)reg_data.volume / volume_divisor;
+  float speaker_volume_ratio = (float)m_speaker_logic.reg_data.volume / volume_divisor;
   unsigned int left_volume = (unsigned int)((128 + vol) * speaker_volume_ratio);
   unsigned int right_volume = (unsigned int)((128 - vol) * speaker_volume_ratio);
 

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.cpp

@@ -307,7 +307,7 @@ static const char* const named_buttons[] = {
 
 void Wiimote::Reset()
 {
-  m_reporting_mode = RT_REPORT_CORE;
+  m_reporting_mode = RT_REPORT_DISABLED;
   m_reporting_channel = 0;
   m_reporting_auto = false;
 
@@ -566,8 +566,7 @@ bool Wiimote::Step()
     // WiiBrew: Following a connection or disconnection event on the Extension Port,
     // data reporting is disabled and the Data Reporting Mode must be reset before new data can
     // arrive.
-    m_reporting_mode = RT_REPORT_CORE;
+    m_reporting_mode = RT_REPORT_DISABLED;
-    m_reporting_auto = false;
 
     RequestStatus();
 
@@ -853,6 +852,13 @@ void Wiimote::Update()
 
   Movie::SetPolledDevice();
 
+  if (RT_REPORT_DISABLED == m_reporting_mode)
+  {
+    // The wiimote is in this disabled state on boot and after an extension change.
+    // Input reports are not sent, even on button change.
+    return;
+  }
+
   const ReportFeatures& rptf = reporting_mode_features[m_reporting_mode - RT_REPORT_CORE];
   s8 rptf_size = rptf.total_size;
   if (Movie::IsPlayingInput() &&
@@ -863,7 +869,7 @@ void Wiimote::Update()
   }
   else
   {
-    data[0] = 0xA1;
+    data[0] = HID_TYPE_DATA << 4 | HID_PARAM_INPUT;
     data[1] = m_reporting_mode;
 
     const auto lock = GetStateLock();
@@ -895,25 +901,25 @@ void Wiimote::Update()
     UpdateIRData(rptf.accel_size != 0);
     if (rptf.ir_size)
     {
-      // TODO: kill magic numbers
+      m_i2c_bus.BusRead(IRCameraLogic::DEVICE_ADDR, offsetof(IRCameraLogic::RegData, camera_data),
-      m_i2c_bus.BusRead(0x58, 0x37, rptf.ir_size, feature_ptr);
+                        rptf.ir_size, feature_ptr);
       feature_ptr += rptf.ir_size;
     }
 
+    // motion plus
+    auto* mplus_data =
+        reinterpret_cast<wm_motionplus_data*>(m_motion_plus_logic.reg_data.controller_data);
+    *mplus_data = wm_motionplus_data();
+    mplus_data->is_mp_data = true;
+
     // extension
     UpdateExtData();
     if (rptf.ext_size)
     {
-      // TODO: kill magic numbers
+      m_i2c_bus.BusRead(ExtensionLogic::DEVICE_ADDR, 0x00, rptf.ext_size, feature_ptr);
-      m_i2c_bus.BusRead(0x52, 0x00, rptf.ext_size, feature_ptr);
       feature_ptr += rptf.ext_size;
     }
 
-    // motion plus
-    auto* mplus_data = reinterpret_cast<wm_motionplus_data*>(m_motion_plus_logic.reg_data.controller_data);
-    *mplus_data = wm_motionplus_data();
-    mplus_data->is_mp_data = true;
-
     if (feature_ptr != data + rptf_size)
     {
       PanicAlert("Wiimote input report is the wrong size!");

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h

@@ -6,6 +6,7 @@
 
 #include <array>
 #include <string>
+#include <algorithm>
 
 #include "Common/Logging/Log.h"
 #include "Core/HW/WiimoteCommon/WiimoteHid.h"
@@ -215,7 +216,6 @@ enum
 class I2CSlave
 {
 public:
-  // Kill MSVC warning:
   virtual ~I2CSlave() = default;
 
   virtual int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) = 0;
@@ -366,6 +366,7 @@ public:
   void Update();
   void InterruptChannel(u16 channel_id, const void* data, u32 size);
   void ControlChannel(u16 channel_id, const void* data, u32 size);
+  void SpeakerData(const u8* data, int length);
   bool CheckForButtonPress();
   void Reset();
 
@@ -394,7 +395,7 @@ private:
 
   struct IRCameraLogic : public I2CSlave
   {
-    struct
+    struct RegData
     {
       // Contains sensitivity and other unknown data
       u8 data[0x33];
@@ -435,9 +436,9 @@ private:
 
     ControllerEmu::Extension* extension;
 
-  private:
     static const u8 DEVICE_ADDR = 0x52;
 
+  private:
     int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override
     {
       if (DEVICE_ADDR != slave_addr)
@@ -476,6 +477,9 @@ private:
 
   struct SpeakerLogic : public I2CSlave
   {
+    static const u8 DATA_FORMAT_ADPCM = 0x00;
+    static const u8 DATA_FORMAT_PCM = 0x40;
+
     struct
     {
       // Speaker reports result in a write of samples to addr 0x00 (which also plays sound)
@@ -499,8 +503,6 @@ private:
 
     static const u8 DEVICE_ADDR = 0x51;
 
-    void SpeakerData(const u8* data, int length);
-
     int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override
     {
       if (DEVICE_ADDR != slave_addr)
@@ -516,7 +518,7 @@ private:
 
       if (0x00 == addr)
       {
-        SpeakerData(data_in, count);
+        ERROR_LOG(WIIMOTE, "Writing of speaker data to address 0x00 is unimplemented!");
         return count;
       }
       else
@@ -564,12 +566,19 @@ private:
 
     u8 GetPassthroughMode() const { return reg_data.ext_identifier[4]; }
 
+    // TODO: when activated it seems the motion plus continuously reads from the ext port even when not in passthrough mode
+    // 16 bytes it seems
+
+    // It also sends the 0x55 to 0xf0 activation
+    // It also writes 0x00 to slave:0x52 addr:0xfa for some reason
+    // And starts a write to 0xfa but never writes bytes..
+
     int BusRead(u8 slave_addr, u8 addr, int count, u8* data_out) override
     {
       if (IsActive())
       {
-        // TODO: does mplus respond to reads on 0xa6 when activated?
+        // Motion plus does not respond to 0x53 when activated
-        if (ACTIVE_DEVICE_ADDR == slave_addr || INACTIVE_DEVICE_ADDR == slave_addr)
+        if (ACTIVE_DEVICE_ADDR == slave_addr)
           return RawRead(&reg_data, addr, count, data_out);
         else
           return 0;
@@ -590,8 +599,8 @@ private:
     {
       if (IsActive())
       {
-        // TODO: does mplus respond to reads on 0xa6 when activated?
+        // Motion plus does not respond to 0x53 when activated
-        if (ACTIVE_DEVICE_ADDR == slave_addr || INACTIVE_DEVICE_ADDR == slave_addr)
+        if (ACTIVE_DEVICE_ADDR == slave_addr)
         {
           auto const result = RawWrite(&reg_data, addr, count, data_in);
           return result;
@@ -604,7 +613,9 @@ private:
           }
         }
         else
+        {
           return 0;
+        }
       }
       else
       {
@@ -642,8 +653,7 @@ private:
         return extension_port.IsDeviceConnected();
       }
     }
-  }
+  } m_motion_plus_logic;
-  m_motion_plus_logic;
 
   void ReportMode(const wm_report_mode* dr);
   void SendAck(u8 report_id, u8 error_code = 0x0);