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

IOS/KD: Implement NWC24_CHECK_MAIL_NOW #12108

Merged
merged 1 commit into from Sep 3, 2023

Conversation

noahpistilli
Copy link
Member

This PR implements both a scheduler for WiiConnect24 as well as the NWC24_CHECK_MAIL_NOW ioctl.

I have decided to split #11746 into separate PR's for easier reviewing.

@noahpistilli noahpistilli force-pushed the kd-check-mail branch 3 times, most recently from be16cab to 6e13a54 Compare August 15, 2023 01:24
Source/Core/Common/StringUtil.cpp Outdated Show resolved Hide resolved
Source/Core/Common/StringUtil.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/Network/KD/Mail/MailCommon.h Outdated Show resolved Hide resolved
Source/Core/Core/IOS/Network/KD/Mail/WC24Send.h Outdated Show resolved Hide resolved
Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp Outdated Show resolved Hide resolved
Source/Core/Common/HttpRequest.cpp Outdated Show resolved Hide resolved
Source/Core/Common/HttpRequest.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/Network/KD/Mail/WC24Send.cpp Outdated Show resolved Hide resolved
Source/Core/Core/IOS/Network/KD/NetKDRequest.cpp Outdated Show resolved Hide resolved
@noahpistilli noahpistilli force-pushed the kd-check-mail branch 2 times, most recently from 4f825c0 to 06433d5 Compare August 19, 2023 20:25
Source/Core/Common/HttpRequest.cpp Outdated Show resolved Hide resolved
Source/Core/Common/HttpRequest.cpp Outdated Show resolved Hide resolved
Source/Core/Common/HttpRequest.cpp Outdated Show resolved Hide resolved
Source/Core/Common/StringUtil.h Outdated Show resolved Hide resolved
@noahpistilli noahpistilli force-pushed the kd-check-mail branch 2 times, most recently from 38bf9ac to 990c9df Compare August 22, 2023 14:01
@noahpistilli noahpistilli force-pushed the kd-check-mail branch 2 times, most recently from c937615 to cc9ecf7 Compare August 22, 2023 15:19
@@ -173,6 +180,17 @@ void HttpRequest::Impl::FollowRedirects(long max)
curl_easy_setopt(m_curl.get(), CURLOPT_MAXREDIRS, max);
}

std::string HttpRequest::Impl::GetHeaderValue(std::string_view name) const
{
for (const auto& [key, value] : m_response_headers)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a slight optimization could be done here? If m_response_headers supported heterogeneous lookup you could use the name as the key in the map. I don't think this is blocking but something to consider.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. Untested

Comment on lines +21 to +22
if (!file || !file->Read(&m_data, 1))
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the file not existing is normal operation (eg. on initial system menu boot), but if it exists but isn't large enough should there be a error log? We have a lot of users with questionable NANDs out there...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There probably should, if the file is corrupted the Wii Menu will display the System Files are corrupted error on any attempt to launch the message board.

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested, but this looks pretty good to me now. Is there a good way to test this as-is?

@noahpistilli
Copy link
Member Author

You would have to use a NAND that is patched with RiiConnect24 mail, as well as a friend linked who can send you mail.

This link can patch if you supply the nwc24msg.cfg file.

I would have recommended WiiLink but their patcher is dependant on #11906.

@noahpistilli
Copy link
Member Author

On another note, I am thinking that we should block starting the scheduler if the user is using the default console ID.

@AdmiralCurtiss
Copy link
Contributor

Yes, blocking the Mail features for the default NAND makes sense.

@noahpistilli
Copy link
Member Author

On another note, I am thinking that we should block starting the scheduler if the user is using the default console ID.

I wanted to future proof this for when we support downloading in the scheduler, so instead of blocking the scheduler starting, I just block dispatching the mail task.

@AdmiralCurtiss
Copy link
Contributor

Tangential to this PR technically, but probably worth fixing before we do threading here...

// On a real Wii, GetSchedulerStat copies memory containing a list of error codes recorded by
// KD among other things. In most instances there will never be more than one error code
// recorded as we do not have a scheduler.
const u32 out_size = std::min(request.buffer_out_size, 256U);
memory.CopyToEmu(request.buffer_out, m_scheduler_buffer.data(), out_size);

This also doesn't seem to lock the m_scheduler_buffer_lock. The comment is also likely outdated with this PR.

@noahpistilli noahpistilli force-pushed the kd-check-mail branch 2 times, most recently from 627395e to 55ad993 Compare September 3, 2023 16:54
@AdmiralCurtiss
Copy link
Contributor

Okay, I think I'm good now. I don't have a setup for testing this so if you don't mind, please give it a go after the recent changes to see if everything still works as expected from your perspective, and if yes give me a poke to merge.

@AdmiralCurtiss AdmiralCurtiss merged commit 143a136 into dolphin-emu:master Sep 3, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants