Address some review comments

src/citra_qt/main.cpp

@@ -1278,13 +1278,8 @@ void GMainWindow::OnLoadAmiibo() {
         Service::SM::ServiceManager& sm = system.ServiceManager();
         auto nfc = sm.GetService<Service::NFC::Module::Interface>("nfc:u");
         if (nfc != nullptr) {
-            auto nfc_module = nfc->GetModule();
+            nfc->LoadAmiibo(filename.toStdString());
-            if (nfc_module != nullptr) {
+            ui.action_Remove_Amiibo->setEnabled(true);
-                nfc_module->nfc_filename = filename.toStdString();
-                nfc_module->nfc_tag_state = Service::NFC::TagState::TagInRange;
-                nfc_module->tag_in_range_event->Signal();
-                ui.action_Remove_Amiibo->setEnabled(true);
-            }
         }
     }
 }
@@ -1294,13 +1289,8 @@ void GMainWindow::OnRemoveAmiibo() {
     Service::SM::ServiceManager& sm = system.ServiceManager();
     auto nfc = sm.GetService<Service::NFC::Module::Interface>("nfc:u");
     if (nfc != nullptr) {
-        auto nfc_module = nfc->GetModule();
+        nfc->RemoveAmiibo();
-        if (nfc_module != nullptr) {
+        ui.action_Remove_Amiibo->setEnabled(false);
-            nfc_module->nfc_filename = "";
-            nfc_module->nfc_tag_state = Service::NFC::TagState::TagOutOfRange;
-            nfc_module->tag_out_of_range_event->Signal();
-            ui.action_Remove_Amiibo->setEnabled(false);
-        }
     }
 }
 

src/core/hle/service/nfc/nfc.cpp

@@ -34,16 +34,6 @@ struct AmiiboConfig {
 };
 static_assert(sizeof(AmiiboConfig) == 0x40, "AmiiboConfig is an invalid size");
 
-struct IdentificationBlockRaw {
-    u16_le char_id;
-    u8 char_variant;
-    u8 figure_type;
-    u16_be model_number;
-    u8 series;
-    INSERT_PADDING_BYTES(0x2F);
-};
-static_assert(sizeof(IdentificationBlockRaw) == 0x36, "IdentificationBlockRaw is an invalid size");
-
 struct IdentificationBlockReply {
     u16_le char_id;
     u8 char_variant;
@@ -107,15 +97,23 @@ void Module::Interface::StartTagScanning(Kernel::HLERequestContext& ctx) {
 void Module::Interface::GetTagInfo(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp(ctx, 0x11, 0, 0);
 
+    ResultCode result = RESULT_SUCCESS;
+    if (nfc->nfc_tag_state != TagState::TagInRange &&
+        nfc->nfc_tag_state != TagState::TagDataLoaded && nfc->nfc_tag_state != TagState::Unknown6) {
+        result = ResultCode(ErrCodes::CommandInvalidForState, ErrorModule::NFC,
+                            ErrorSummary::InvalidState, ErrorLevel::Status);
+    }
+
     TagInfo tag_info{};
-    auto nfc_file = FileUtil::IOFile(nfc->nfc_filename, "rb");
+    nfc->amiibo_data_mutex.lock();
-    nfc_file.ReadBytes(tag_info.uuid.data(), sizeof(tag_info.uuid.size()));
+    tag_info.uuid = nfc->amiibo_data.uuid;
-    tag_info.id_offset_size = 0x7;
+    tag_info.id_offset_size = tag_info.uuid.size();
+    nfc->amiibo_data_mutex.unlock();
     tag_info.unk1 = 0x0;
     tag_info.unk2 = 0x2;
 
     IPC::RequestBuilder rb = rp.MakeBuilder(12, 0);
-    rb.Push(RESULT_SUCCESS);
+    rb.Push(result);
     rb.PushRaw<TagInfo>(tag_info);
     LOG_WARNING(Service_NFC, "(STUBBED) called");
 }
@@ -188,7 +186,7 @@ void Module::Interface::GetTagState(Kernel::HLERequestContext& ctx) {
 
     IPC::RequestBuilder rb = rp.MakeBuilder(2, 0);
     rb.Push(RESULT_SUCCESS);
-    rb.PushEnum(nfc->nfc_tag_state);
+    rb.PushEnum(nfc->nfc_tag_state.load());
     LOG_DEBUG(Service_NFC, "called");
 }
 
@@ -204,31 +202,39 @@ void Module::Interface::CommunicationGetStatus(Kernel::HLERequestContext& ctx) {
 void Module::Interface::Unknown0x1A(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp(ctx, 0x1A, 0, 0);
 
-    nfc->nfc_tag_state = TagState::Unknown6;
+    ResultCode result = RESULT_SUCCESS;
+    if (nfc->nfc_tag_state != TagState::TagInRange) {
+        result = ResultCode(ErrCodes::CommandInvalidForState, ErrorModule::NFC,
+                            ErrorSummary::InvalidState, ErrorLevel::Status);
+    } else {
+        nfc->nfc_tag_state = TagState::Unknown6;
+    }
 
     IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);
-    rb.Push(RESULT_SUCCESS);
+    rb.Push(result);
     LOG_DEBUG(Service_NFC, "called");
 }
 
-void Module::Interface::Unknown0x1B(Kernel::HLERequestContext& ctx) {
+void Module::Interface::GetIdentificationBlock(Kernel::HLERequestContext& ctx) {
     IPC::RequestParser rp(ctx, 0x1B, 0, 0);
 
-    IdentificationBlockRaw identification_block_raw{};
+    ResultCode result = RESULT_SUCCESS;
-    auto nfc_file = FileUtil::IOFile(nfc->nfc_filename, "rb");
+    if (nfc->nfc_tag_state != TagState::TagDataLoaded && nfc->nfc_tag_state != TagState::Unknown6) {
-    // go to offset of the Amiibo identification block
+        result = ResultCode(ErrCodes::CommandInvalidForState, ErrorModule::NFC,
-    nfc_file.Seek(0x54, SEEK_SET);
+                            ErrorSummary::InvalidState, ErrorLevel::Status);
-    nfc_file.ReadBytes(&identification_block_raw, 0x7);
+    }
 
     IdentificationBlockReply identification_block_reply{};
-    identification_block_reply.char_id = identification_block_raw.char_id;
+    nfc->amiibo_data_mutex.lock();
-    identification_block_reply.char_variant = identification_block_raw.char_variant;
+    identification_block_reply.char_id = nfc->amiibo_data.char_id;
-    identification_block_reply.series = identification_block_raw.series;
+    identification_block_reply.char_variant = nfc->amiibo_data.char_variant;
-    identification_block_reply.model_number = identification_block_raw.model_number;
+    identification_block_reply.series = nfc->amiibo_data.series;
-    identification_block_reply.figure_type = identification_block_raw.figure_type;
+    identification_block_reply.model_number = nfc->amiibo_data.model_number;
+    identification_block_reply.figure_type = nfc->amiibo_data.figure_type;
+    nfc->amiibo_data_mutex.unlock();
 
     IPC::RequestBuilder rb = rp.MakeBuilder(0x1F, 0);
-    rb.Push(RESULT_SUCCESS);
+    rb.Push(result);
     rb.PushRaw<IdentificationBlockReply>(identification_block_reply);
     LOG_DEBUG(Service_NFC, "called");
 }
@@ -242,6 +248,23 @@ std::shared_ptr<Module> Module::Interface::GetModule() const {
     return nfc;
 }
 
+void Module::Interface::LoadAmiibo(const std::string& filename) {
+    auto nfc_file = FileUtil::IOFile(filename, "rb");
+    nfc->amiibo_data_mutex.lock();
+    nfc_file.ReadBytes(&nfc->amiibo_data, sizeof(AmiiboData));
+    nfc->amiibo_data_mutex.unlock();
+    nfc->nfc_tag_state = Service::NFC::TagState::TagInRange;
+    nfc->tag_in_range_event->Signal();
+}
+
+void Module::Interface::RemoveAmiibo() {
+    nfc->nfc_tag_state = Service::NFC::TagState::TagOutOfRange;
+    nfc->tag_out_of_range_event->Signal();
+    nfc->amiibo_data_mutex.lock();
+    nfc->amiibo_data = {};
+    nfc->amiibo_data_mutex.unlock();
+}
+
 Module::Module() {
     tag_in_range_event =
         Kernel::Event::Create(Kernel::ResetType::OneShot, "NFC::tag_in_range_event");

src/core/hle/service/nfc/nfc.h

@@ -4,7 +4,9 @@
 
 #pragma once
 
+#include <atomic>
 #include <memory>
+#include <mutex>
 #include "common/common_types.h"
 #include "core/hle/kernel/kernel.h"
 #include "core/hle/service/service.h"
@@ -25,6 +27,19 @@ enum {
 };
 } // namespace ErrCodes
 
+// TODO(FearlessTobi): Add more members to this struct
+struct AmiiboData {
+    std::array<u8, 7> uuid;
+    INSERT_PADDING_BYTES(0x4D);
+    u16_le char_id;
+    u8 char_variant;
+    u8 figure_type;
+    u16_be model_number;
+    u8 series;
+    INSERT_PADDING_BYTES(0x1C1);
+};
+static_assert(sizeof(AmiiboData) == 0x21C, "AmiiboData is an invalid size");
+
 enum class TagState : u8 {
     NotInitialized = 0,
     NotScanning = 1,
@@ -52,6 +67,10 @@ public:
 
         std::shared_ptr<Module> GetModule() const;
 
+        void LoadAmiibo(const std::string& filename);
+
+        void RemoveAmiibo();
+
     protected:
         /**
          * NFC::Initialize service function
@@ -200,25 +219,27 @@ public:
         void Unknown0x1A(Kernel::HLERequestContext& ctx);
 
         /**
-         * NFC::Unknown0x1B service function
+         * NFC::GetIdentificationBlock service function
          *  Inputs:
          *      0 : Header code [0x001B0000]
          *  Outputs:
          *      1 : Result of function, 0 on success, otherwise error code
          *   2-31 : 0x36-byte struct
          */
-        void Unknown0x1B(Kernel::HLERequestContext& ctx);
+        void GetIdentificationBlock(Kernel::HLERequestContext& ctx);
 
     private:
         std::shared_ptr<Module> nfc;
     };
 
+private:
     Kernel::SharedPtr<Kernel::Event> tag_in_range_event;
     Kernel::SharedPtr<Kernel::Event> tag_out_of_range_event;
-    TagState nfc_tag_state = TagState::NotInitialized;
+    std::atomic<TagState> nfc_tag_state = TagState::NotInitialized;
     CommunicationStatus nfc_status = CommunicationStatus::NfcInitialized;
 
-    std::string nfc_filename;
+    AmiiboData amiibo_data{};
+    std::mutex amiibo_data_mutex;
 };
 
 void InstallInterfaces(Core::System& system);

src/core/hle/service/nfc/nfc_m.cpp

@@ -34,7 +34,7 @@ NFC_M::NFC_M(std::shared_ptr<Module> nfc) : Module::Interface(std::move(nfc), "n
         {0x00180000, &NFC_M::GetAmiiboConfig, "GetAmiiboConfig"},
         {0x00190000, nullptr, "GetAppDataInitStruct"},
         {0x001A0000, &NFC_M::Unknown0x1A, "Unknown0x1A"},
-        {0x001B0000, &NFC_M::Unknown0x1B, "Unknown0x1B"},
+        {0x001B0000, &NFC_M::GetIdentificationBlock, "GetIdentificationBlock"},
         // nfc:m
         {0x04040A40, nullptr, "SetAmiiboSettings"}
         // clang-format on

src/core/hle/service/nfc/nfc_u.cpp

@@ -33,7 +33,7 @@ NFC_U::NFC_U(std::shared_ptr<Module> nfc) : Module::Interface(std::move(nfc), "n
         {0x00180000, &NFC_U::GetAmiiboConfig, "GetAmiiboConfig"},
         {0x00190000, nullptr, "GetAppDataInitStruct"},
         {0x001A0000, &NFC_U::Unknown0x1A, "Unknown0x1A"},
-        {0x001B0000, &NFC_U::Unknown0x1B, "Unknown0x1B"},
+        {0x001B0000, &NFC_U::GetIdentificationBlock, "GetIdentificationBlock"},
         // clang-format on
     };
     RegisterHandlers(functions);