WiimoteEmu: Process wiimote read data requests like they are on a real wiimote. It's not a queue. New requests are ignored and input is suppressed while processing a request. This simplifies the save state code greatly.

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

@@ -439,7 +439,7 @@ struct wm_read_data_reply
 {
   wm_buttons buttons;
   u8 error : 4;  // see WM_RDERR_*
-  u8 size : 4;
+  u8 size_minus_one : 4;
   u16 address;
   u8 data[16];
 };

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

@@ -119,11 +119,12 @@ void Wiimote::HidOutputReport(const wm_report* const sr, const bool send_ack)
 
   case RT_WRITE_DATA:  // 0x16
     WriteData(reinterpret_cast<const wm_write_data*>(sr->data));
+    return;  // sends its own ack
     break;
 
   case RT_READ_DATA:  // 0x17
     ReadData(reinterpret_cast<const wm_read_data*>(sr->data));
-    return;  // sends its own ack
+    return;  // sends its own ack/reply
     break;
 
   case RT_WRITE_SPEAKER_DATA:  // 0x18
@@ -167,7 +168,7 @@ void Wiimote::HidOutputReport(const wm_report* const sr, const bool send_ack)
    The first two bytes are the core buttons data,
    00 00 means nothing is pressed.
    The last byte is the success code 00. */
-void Wiimote::SendAck(u8 report_id)
+void Wiimote::SendAck(u8 report_id, u8 error_code)
 {
   u8 data[6];
 
@@ -178,7 +179,7 @@ void Wiimote::SendAck(u8 report_id)
 
   ack->buttons = m_status.buttons;
   ack->reportID = report_id;
-  ack->errorID = 0;
+  ack->errorID = error_code;
 
   Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, data, sizeof(data));
 }
@@ -235,6 +236,8 @@ void Wiimote::WriteData(const wm_write_data* const wd)
     return;
   }
 
+  u8 error_code = 0;
+
   switch (wd->space)
   {
   case WS_EEPROM:
@@ -267,13 +270,15 @@ void Wiimote::WriteData(const wm_write_data* const wd)
   {
     // Write to Control Register
 
-    // TODO: generate a writedata error reply, 7 == no such slave (no ack)
+    // Top byte of address is ignored on the bus.
-    m_i2c_bus.BusWrite(wd->slave_address >> 1, address & 0xff, wd->size, wd->data);
+    auto const bytes_written = m_i2c_bus.BusWrite(wd->slave_address >> 1, (u8)address, wd->size, wd->data);
+    if (bytes_written != wd->size)
+    {
+      // A real wiimote gives error 7 for failed write to i2c bus (mainly a non-existant slave)
+      error_code = 0x07;
+    }
 
-    return;
+    // else if (&m_reg_motion_plus == region_ptr)
-
-
-    //else if (&m_reg_motion_plus == region_ptr)
     //{
     //  // activate/deactivate motion plus
     //  if (0x55 == m_reg_motion_plus.activated)
@@ -291,50 +296,95 @@ void Wiimote::WriteData(const wm_write_data* const wd)
     PanicAlert("WriteData: unimplemented parameters!");
     break;
   }
+
+  SendAck(RT_WRITE_DATA, error_code);
 }
 
 /* Read data from Wiimote and Extensions registers. */
 void Wiimote::ReadData(const wm_read_data* const rd)
 {
-  u16 address = Common::swap16(rd->address);
+  if (m_read_request.size)
-  u16 size = Common::swap16(rd->size);
+  {
+    // There is already an active read request.
+    // a real wiimote ignores the new one.
+    return;
+  }
 
-  //INFO_LOG(WIIMOTE, "Wiimote::ReadData: %d @ 0x%02x @ 0x%02x (%d)", rd->space, rd->slave_address, address, size);
+  // Save the request and process it on the next "Update()" calls
+  m_read_request.space = rd->space;
+  m_read_request.slave_address = rd->slave_address;
+  m_read_request.address = Common::swap16(rd->address);
+  // A zero size request is just ignored, like on the real wiimote.
+  m_read_request.size = Common::swap16(rd->size);
 
-  ReadRequest rr;
+  INFO_LOG(WIIMOTE, "Wiimote::ReadData: %d @ 0x%02x @ 0x%02x (%d)", m_read_request.space,
-  u8* const block = new u8[size];
+          m_read_request.slave_address, m_read_request.address, m_read_request.size);
 
-  switch (rd->space)
+  // Send up to one read-data-reply.
+  // If more data needs to be sent it will happen on the next "Update()"
+  ProcessReadDataRequest();
+}
+
+bool Wiimote::ProcessReadDataRequest()
+{
+  // Limit the amt to 16 bytes
+  // AyuanX: the MTU is 640B though... what a waste!
+  u16 const bytes_to_read = std::min((u16)16, m_read_request.size);
+
+  if (0 == bytes_to_read)
+  {
+    // No active request:
+    return false;
+  }
+
+  u8 data[23] = {};
+  data[0] = 0xA1;
+  data[1] = RT_READ_DATA_REPLY;
+
+  wm_read_data_reply* const reply = reinterpret_cast<wm_read_data_reply*>(data + 2);
+  reply->buttons = m_status.buttons;
+  reply->address = Common::swap16(m_read_request.address);
+
+  switch (m_read_request.space)
   {
   case WS_EEPROM:
   {
     // Read from EEPROM
-    if (address + size >= WIIMOTE_EEPROM_FREE_SIZE)
+    if (m_read_request.address + m_read_request.size >= WIIMOTE_EEPROM_FREE_SIZE)
     {
-      if (address + size > WIIMOTE_EEPROM_SIZE)
+      if (m_read_request.address + m_read_request.size > WIIMOTE_EEPROM_SIZE)
       {
         PanicAlert("ReadData: address + size out of bounds");
-        delete[] block;
-        return;
       }
 
-      // generate a read error, even if the start of the block is readable a real wiimote just sends error code 8
+      // generate a read error, even if the start of the block is readable a real wiimote just sends
-      size = 0;
+      // error code 8
-    }
 
-    // read mii data from file
+      // The real Wiimote generate an error for the first
-    if (address >= 0x0FCA && address < 0x12C0)
+      // request to 0x1770 if we dont't replicate that the game will never
+      // read the calibration data at the beginning of Eeprom. I think this
+      // error is supposed to occur when we try to read above the freely
+      // usable space that ends at 0x16ff.
+      reply->error = 0x08;
+    }
+    else
     {
-      // TODO Only read the Mii block parts required
+      // Mii block handling:
-      std::ifstream file;
+      // TODO: different filename for each wiimote?
-      File::OpenFStream(file, (File::GetUserPath(D_SESSION_WIIROOT_IDX) + "/mii.bin").c_str(),
+      if (m_read_request.address >= 0x0FCA && m_read_request.address < 0x12C0)
-                        std::ios::binary | std::ios::in);
+      {
-      file.read((char*)m_eeprom + 0x0FCA, 0x02f0);
+        // TODO: Only read the Mii block parts required
-      file.close();
+        std::ifstream file;
-    }
+        File::OpenFStream(file, (File::GetUserPath(D_SESSION_WIIROOT_IDX) + "/mii.bin").c_str(),
+                          std::ios::binary | std::ios::in);
+        file.read((char*)m_eeprom + 0x0FCA, 0x02f0);
+        file.close();
+      }
 
-    // read memory to be sent to Wii
+      // read memory to be sent to Wii
-    memcpy(block, m_eeprom + address, size);
+      std::copy_n(m_eeprom + m_read_request.address, bytes_to_read, reply->data);
+      reply->size_minus_one = bytes_to_read - 1;
+    }
   }
   break;
 
@@ -343,83 +393,33 @@ void Wiimote::ReadData(const wm_read_data* const rd)
   {
     // Read from Control Register
 
-    m_i2c_bus.BusRead(rd->slave_address >> 1, address & 0xff, size, block);
+    // Top byte of address is ignored on the bus, but it IS maintained in the read-reply.
-    // TODO: generate read errors, 7 == no such slave (no ack)
+    auto const bytes_read = m_i2c_bus.BusRead(m_read_request.slave_address >> 1,
+                                        (u8)m_read_request.address, bytes_to_read, reply->data);
+
+    reply->size_minus_one = bytes_read - 1;
+
+    if (bytes_read != bytes_to_read)
+    {
+      // generate read error, 7 == no such slave (no ack)
+      reply->error = 0x07;
+    }
   }
   break;
 
   default:
-    PanicAlert("Wiimote::ReadData: unimplemented address space (space: 0x%x)!", rd->space);
+    PanicAlert("Wiimote::ReadData: unimplemented address space (space: 0x%x)!", m_read_request.space);
     break;
   }
 
-  rr.address = address;
+  // Modify the read request, zero size == complete
-  rr.size = size;
+  m_read_request.address += bytes_to_read;
-  rr.position = 0;
+  m_read_request.size -= bytes_to_read;
-  rr.data = block;
 
-  // TODO: read requests suppress normal input reports
+  // Send the data
-  // TODO: if there is currently an active read request ignore new ones
-
-  // send up to 16 bytes
-  SendReadDataReply(rr);
-
-  // if there is more data to be sent, add it to the queue
-  if (rr.size)
-    m_read_requests.push(rr);
-  else
-    delete[] rr.data;
-}
-
-void Wiimote::SendReadDataReply(ReadRequest& request)
-{
-  u8 data[23];
-  data[0] = 0xA1;
-  data[1] = RT_READ_DATA_REPLY;
-
-  wm_read_data_reply* const reply = reinterpret_cast<wm_read_data_reply*>(data + 2);
-  reply->buttons = m_status.buttons;
-  reply->address = Common::swap16(request.address);
-
-  // generate a read error
-  // Out of bounds. The real Wiimote generate an error for the first
-  // request to 0x1770 if we dont't replicate that the game will never
-  // read the calibration data at the beginning of Eeprom. I think this
-  // error is supposed to occur when we try to read above the freely
-  // usable space that ends at 0x16ff.
-  if (0 == request.size)
-  {
-    reply->size = 0x0f;
-    reply->error = 0x08;
-
-    memset(reply->data, 0, sizeof(reply->data));
-  }
-  else
-  {
-    // Limit the amt to 16 bytes
-    // AyuanX: the MTU is 640B though... what a waste!
-    const int amt = std::min((u16)16, request.size);
-
-    // no error
-    reply->error = 0;
-
-    // 0x1 means two bytes, 0xf means 16 bytes
-    reply->size = amt - 1;
-
-    // Clear the mem first
-    memset(reply->data, 0, sizeof(reply->data));
-
-    // copy piece of mem
-    memcpy(reply->data, request.data + request.position, amt);
-
-    // update request struct
-    request.size -= amt;
-    request.position += amt;
-    request.address += amt;
-  }
-
-  // Send a piece
   Core::Callback_WiimoteInterruptChannel(m_index, m_reporting_channel, data, sizeof(data));
+
+  return true;
 }
 
 void Wiimote::DoState(PointerWrap& p)
@@ -446,47 +446,7 @@ void Wiimote::DoState(PointerWrap& p)
   p.Do(m_camera_logic.reg_data);
   p.Do(m_ext_logic.reg_data);
   p.Do(m_speaker_logic.reg_data);
-
+  p.Do(m_read_request);
-  // Do 'm_read_requests' queue
-  {
-    u32 size = 0;
-    if (p.mode == PointerWrap::MODE_READ)
-    {
-      // clear
-      while (!m_read_requests.empty())
-      {
-        delete[] m_read_requests.front().data;
-        m_read_requests.pop();
-      }
-
-      p.Do(size);
-      while (size--)
-      {
-        ReadRequest tmp;
-        p.Do(tmp.address);
-        p.Do(tmp.position);
-        p.Do(tmp.size);
-        tmp.data = new u8[tmp.size];
-        p.DoArray(tmp.data, tmp.size);
-        m_read_requests.push(tmp);
-      }
-    }
-    else
-    {
-      std::queue<ReadRequest> tmp_queue(m_read_requests);
-      size = (u32)(m_read_requests.size());
-      p.Do(size);
-      while (!tmp_queue.empty())
-      {
-        ReadRequest tmp = tmp_queue.front();
-        p.Do(tmp.address);
-        p.Do(tmp.position);
-        p.Do(tmp.size);
-        p.DoArray(tmp.data, tmp.size);
-        tmp_queue.pop();
-      }
-    }
-  }
   p.DoMarker("Wiimote");
 
   if (p.GetMode() == PointerWrap::MODE_READ)
@@ -504,4 +464,4 @@ void Wiimote::RealState()
     g_wiimotes[m_index]->EnableDataReporting(m_reporting_mode);
   }
 }
-}
+}  // namespace WiimoteEmu

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

@@ -353,12 +353,7 @@ void Wiimote::Reset()
   m_swing_dynamic_data = {};
   m_shake_dynamic_data = {};
 
-  // clear read request queue
+  m_read_request.size = 0;
-  while (!m_read_requests.empty())
-  {
-    delete[] m_read_requests.front().data;
-    m_read_requests.pop();
-  }
 
   // Yamaha ADPCM state initialize
   m_speaker_logic.adpcm_state.predictor = 0;
@@ -552,21 +547,10 @@ bool Wiimote::Step()
     UpdateButtonsStatus();
   }
 
-  // check if there is a read data request
+  if (ProcessReadDataRequest())
-  if (!m_read_requests.empty())
   {
-    ReadRequest& rr = m_read_requests.front();
+    // Read requests suppress normal input reports
-    // send up to 16 bytes to the Wii
+    // Don't send any other reports
-    SendReadDataReply(rr);
-
-    // if there is no more data, remove from queue
-    if (0 == rr.size)
-    {
-      delete[] rr.data;
-      m_read_requests.pop();
-    }
-
-    // don't send any other reports
     return true;
   }
 
@@ -923,7 +907,8 @@ void Wiimote::Update()
       feature_ptr += rptf.ext;
     }
 
-    Movie::CallWiiInputManip(data, rptf, m_index, m_extension->active_extension, m_ext_logic.ext_key);
+    Movie::CallWiiInputManip(data, rptf, m_index, m_extension->active_extension,
+                             m_ext_logic.ext_key);
   }
   if (NetPlay::IsNetPlayRunning())
   {
@@ -933,7 +918,8 @@ void Wiimote::Update()
   }
 
   // TODO: need to fix usage of rptf probably
-  Movie::CheckWiimoteStatus(m_index, data, rptf, m_extension->active_extension, m_ext_logic.ext_key);
+  Movie::CheckWiimoteStatus(m_index, data, rptf, m_extension->active_extension,
+                            m_ext_logic.ext_key);
 
   // don't send a data report if auto reporting is off
   if (false == m_reporting_auto && data[1] >= RT_REPORT_CORE)

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

@@ -5,7 +5,6 @@
 #pragma once
 
 #include <array>
-#include <queue>
 #include <string>
 
 #include "Common/Logging/Log.h"
@@ -220,6 +219,8 @@ public:
   {
     static_assert(std::is_pod<T>::value);
 
+    // TODO: addr wraps around after 0xff
+
     u8* src = reinterpret_cast<u8*>(reg_data) + addr;
     count = std::min(count, int(reinterpret_cast<u8*>(reg_data + 1) - src));
 
@@ -233,6 +234,8 @@ public:
   {
     static_assert(std::is_pod<T>::value);
 
+    // TODO: addr wraps around after 0xff
+
     u8* dst = reinterpret_cast<u8*>(reg_data) + addr;
     count = std::min(count, int(reinterpret_cast<u8*>(reg_data + 1) - dst));
 
@@ -440,18 +443,12 @@ private:
 
   } m_speaker_logic;
 
-  struct ReadRequest
-  {
-    u16 address, size, position;
-    u8* data;
-  };
-
   void ReportMode(const wm_report_mode* dr);
-  void SendAck(u8 report_id);
+  void SendAck(u8 report_id, u8 error_code = 0x0);
   void RequestStatus(const wm_request_status* rs = nullptr);
   void ReadData(const wm_read_data* rd);
   void WriteData(const wm_write_data* wd);
-  void SendReadDataReply(ReadRequest& request);
+  bool ProcessReadDataRequest();
   bool NetPlay_GetWiimoteData(int wiimote, u8* data, u8 size, u8 reporting_mode);
 
   // control groups
@@ -505,10 +502,13 @@ private:
 
   wm_status_report m_status;
 
-  // read data request queue
+  struct ReadRequest
-  // maybe it isn't actually a queue
+  {
-  // maybe read requests cancel any current requests
+    u8 space;
-  std::queue<ReadRequest> m_read_requests;
+    u8 slave_address;
+    u16 address;
+    u16 size;
+  } m_read_request;
 
 #pragma pack(push, 1)
   u8 m_eeprom[WIIMOTE_EEPROM_SIZE];