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

Add Protocol.Payload and related, add Protocol.Payloads all except AddrPayload, VersionPayload #55

Closed
wants to merge 1 commit into from

Conversation

fonix232
Copy link
Contributor

No description provided.

@fonix232 fonix232 mentioned this pull request Dec 12, 2014
18 tasks
@NicolasDorier
Copy link
Collaborator

Why porting Payloads ? Because socket is not available, any direct network connection can't be done. It is the reason why I decided to now port Payloads.
Are you in a profile having sockets ? or specific plateform ? if yes, which one ?

@fonix232
Copy link
Contributor Author

On Profile111/136, you can manually import C:\Windows\System32\WinMetadata\Windows.Networking.winmd - which has a few socket implementations.

They are 1, not as generic as System.Net.Sockets.Socket 2, not available on other platforms, but it is something, and definitely more than nothing.

@NicolasDorier
Copy link
Collaborator

Not having windows 8 I am blind right now.
Does the definition of this other Socket class available somewhere ? (or can you forward me the winmd ?)

@NicolasDorier
Copy link
Collaborator

I will probably merge it, only tomorrow I will work on another approach to create the portable projects.
Duplicating files like that is not the right approach and error prone.

If I don't find a good enough solution tomorrow, I'll merge that.

@fonix232
Copy link
Contributor Author

I know it is not the right approach, and the only duplications are so that they can be analyzed and the proper code parts can be put into an #ifdef (I was way too lazy to do it myself).

The Sockets available are detailed here: http://www.drdobbs.com/windows/the-new-socket-apis-in-windows-8/240148403?pgno=2

They are quite limited - only TCP and UDP are supported, no raw; can only run when the app is focused, and there are a few other limitations.

@NicolasDorier
Copy link
Collaborator

shit, with their socket Async is mandatory, it would break the whole code.
Do you know if microsoft would refuse the app on the store if we use blocking calls on secondary thread ?

@fonix232
Copy link
Contributor Author

I'm not sure. But why not define separate methods for WP/WinRT and Android/iOS/General?
The separation is definitely possible, and then we would have the WP/WinRT specific async functions, and the other platform non-async ones.

Something like this:

public
#ifdef PLATFORM_WIN
async Task<ReturnValue>
#else
ReturnValue
#endif
GetReturnValue(int inValue) 
{
    #if PLATFORM_WIN
        ReturnValue value = await AnotherTask(intValue);
    #else
        ReturnValue value = AnotherTask(intValue);
    #endif

    return value;
}

@NicolasDorier
Copy link
Collaborator

Since all plateforms support async, there is no reason not to provide them for everywhere.

By the way, I worked a lot today on a better way to create portable libraries, so we don't have any duplication, and everything stay tested. I will soon push that :)

The android version will support network connection, since it supports sockets

@NicolasDorier
Copy link
Collaborator

I close this pull request.
As I said at #56, I modified the portable project to include all of that (and more) without any code duplication.

@fonix232
Copy link
Contributor Author

Then I do not see the problem why not to implement the Windows sockets?

@NicolasDorier
Copy link
Collaborator

The code have a strong dependency on the berkeley style Socket class.
Using the WinRT version instead will break all the code because of semantic change over berkeley's.

This is definitively possible, but the code can't be portable and must be winrt specific. Also, it will not benefit from my unit tests.

If you want to try doing a WinRT specific version of it, I'll be glad to help you. But I don't have windows 8, so I will not be able to compile that myself.

One possibility is to do a Berkeley Socket class implementation that use the WinRT socket internally, but I think this is a very hard job, but it would make the code portable.

@fonix232
Copy link
Contributor Author

Wouldn't it be possible to add Windows-specific #if parts for the classes that use the Berkeley socket? Same class and method structure, but the internal part of the classes would differ.

@NicolasDorier
Copy link
Collaborator

The problem is that it is not same method structure.

Task change the game a lot. Also I have a dependency on Thread, which I'm not sure is available.
Worth trying, but I don't think this is easy stuff.

dalijolijo pushed a commit to dalijolijo/NBitcoin that referenced this pull request Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants