Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nfc: Add Amiibo support (REOPENED) #4337

Merged
merged 8 commits into from Oct 25, 2018

Conversation

Projects
None yet
3 participants
@FearlessTobi
Copy link
Contributor

FearlessTobi commented Oct 13, 2018

THIS PR HAD TO BE REMADE CAUSE I ACCIDENTALLY FORCE-PUSHED TO MASTER.

This PR adds the ability to use "virtual Amiibo" files to emulate the scanning of them.
Currently no keys are needed cause the encrypted sections are not accessed.

UI screenshot:
unbenannt

Example:
grafik


This change is Reviewable

@FearlessTobi FearlessTobi changed the title nfc: Add Amiibo support nfc: Add Amiibo support (REOPENED) Oct 13, 2018

@FearlessTobi
Copy link
Contributor Author

FearlessTobi left a comment

Unaddressed review comments from #4333.

void Module::Interface::GetAmiiboConfig(Kernel::HLERequestContext& ctx) {
IPC::RequestParser rp(ctx, 0x18, 0, 0);

AmiiboConfig amiibo_config{};

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
B3n30 a day ago

Check the state and error if it doesn't match
IPC::RequestParser rp(ctx, 0x1A, 0, 0);

if (nfc->nfc_tag_state != TagState::TagInRange) {
IPC::RequestBuilder rb = rp.MakeBuilder(1, 0);

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
B3n30 9 hours ago

Move IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); before the if and remove to later one
if (nfc != nullptr) {
Service::NFC::AmiiboData amiibo_data{};
auto nfc_file = FileUtil::IOFile(filename.toStdString(), "rb");
nfc_file.ReadBytes(&amiibo_data, sizeof(Service::NFC::AmiiboData));

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
B3n30 9 hours ago Member

check that the result returned match the requested size and LOG_ERROR; return if not
<string>Amiibo</string>
</property>
<addaction name="action_Load_Amiibo"/>
<addaction name="action_Remove_Amiibo"/>

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
zhaowenlan1779 8 hours ago

this isn't properly indented
<property name="text">
<string>Remove</string>
</property>
</action>

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
zhaowenlan1779 8 hours ago

ditto. 1-space please.
nfc->amiibo_data_mutex.lock();
tag_info.uuid = nfc->amiibo_data.uuid;
tag_info.id_offset_size = tag_info.uuid.size();
nfc->amiibo_data_mutex.unlock();

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
zhaowenlan1779 8 hours ago

I think it's preferred to use std::lock_guard here, so that the mutex still got unlocked in case any exception raised.
identification_block_reply.series = nfc->amiibo_data.series;
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();

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
zhaowenlan1779 8 hours ago

ditto here and below.
void GMainWindow::OnLoadAmiibo() {
const QString extensions{"*.bin"};
const QString file_filter =
tr("Amiibo File") + " (" + extensions + ");;" + tr("All Files (*.*)");

This comment has been minimized.

Copy link
@FearlessTobi

FearlessTobi Oct 13, 2018

Author Contributor
zhaowenlan1779 8 hours ago

It may be more preferable to use tr("Amiibo File (%1);; All Files (*.*)").arg(extensions) here
@wwylele
Copy link
Member

wwylele left a comment

looks good other than these comments

Show resolved Hide resolved src/core/hle/service/nfc/nfc.cpp
Show resolved Hide resolved src/core/hle/service/nfc/nfc.cpp Outdated
}

void Module::Interface::RemoveAmiibo() {
HLE::g_hle_lock.lock();

This comment has been minimized.

Copy link
@B3n30

B3n30 Oct 17, 2018

Contributor
Suggested change
HLE::g_hle_lock.lock();
std::lock_guardstd::mutex lock(HLE::g_hle_lock);

@FearlessTobi FearlessTobi force-pushed the FearlessTobi:amiibo branch from 96a332b to c414b30 Oct 17, 2018

#include <memory>
#include <mutex>

This comment has been minimized.

Copy link
@wwylele

wwylele Oct 19, 2018

Member

unused header

FearlessTobi added some commits Oct 19, 2018

@wwylele wwylele merged commit dec3fb2 into citra-emu:master Oct 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@FearlessTobi FearlessTobi deleted the FearlessTobi:amiibo branch Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.