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

Windows support #9

Closed
lvsecoto opened this issue Feb 7, 2022 · 19 comments
Closed

Windows support #9

lvsecoto opened this issue Feb 7, 2022 · 19 comments
Labels
enhancement New feature or request

Comments

@lvsecoto
Copy link

lvsecoto commented Feb 7, 2022

Thank! This plugin is great.

And I also need Windows support.

I think it would be difficult...

Here are documents of Windows API. Thanks!

win32
https://docs.microsoft.com/en-us/windows/win32/api/windns/nf-windns-dnsservicebrowse
https://docs.microsoft.com/en-us/windows/win32/api/windns/nf-windns-dnsserviceregister

uwp
https://docs.microsoft.com/en-us/uwp/api/windows.networking.servicediscovery.dnssd

@sebastianhaberey
Copy link
Owner

Windows support is definitely the next big feature I'd like to implement. Unfortunately I don't have access to a Windows machine right now (I might have by end of March).

@sebastianhaberey sebastianhaberey added the enhancement New feature or request label Feb 7, 2022
@lvsecoto
Copy link
Author

lvsecoto commented Feb 7, 2022

Windows support is definitely the next big feature I'd like to implement. Unfortunately I don't have access to a Windows machine right now (I might have by end of March).

Thank you~ I am look forward to your big feature~

@lxp-git
Copy link

lxp-git commented Feb 10, 2022

I am trying to finish it. No error handling. Code clean is needed also. I will do that later.

https://github.com/lxp-git/nsd/commit/fa2827d2e2a6c079c35080c8b2689b2899a45847

@sebastianhaberey
Copy link
Owner

That's good news! If you like, you can publish your nsd_windows as endorsed federated plugin once you're done. It's pretty easy to do, if you have an account on pub.dev. If it passes the integration test, I'll endorse it by adding the dependency to the plugin. That way, you'll be the maintainer of nsd_windows in your own repository, and the plugin will support windows.

@lxp-git
Copy link

lxp-git commented Feb 12, 2022

I published my first version on Pub. Some issues still have.

  1. onServiceDiscovered callback repeatly.
  2. No onServiceLost found in native windows API.

I will continue to work hard.

@sebastianhaberey
Copy link
Owner

Great! Have you tried running the integration tests with it? They should run out of the box, since they are platform independent. In Android Studio, right click nsd > example > integration_test > test.dart and select Run. This should run the existing integration tests on whatever platform is selected in the IDE.

Once the example application works as intended and the integration tests pass, I'll gladly add the dependency to the main project.

Also, I'll try to set up the integration tests for Windows using Github actions, as I did for the other platforms. This will enable me to run the integration test on the CI whenever a new version of your plugin is released.

@sebastianhaberey
Copy link
Owner

Sorry, I did not take into account that there needs to be a Windows runner in the example application too. So I've created a new branch nsd_windows_endorse that has a basic Windows runner created with flutter create.

You can see the result of running the integration test against version 1.0.0 here.

As you can see, the test does not compile currently. I am convinced this requires some minor tweaking on my side, in nsd/example/windows. Unfortunately I cannot fix this, since I don't have Windows.

I suppose that your nsd/example/windows is already working? If so, I'll gladly take any suggestions / pull request for this directory.

@lxp-git
Copy link

lxp-git commented Feb 12, 2022

@sebastianhaberey

  1. I checkout your branch and tried running the integration tests. It works well. Six tests
  2. I noticed that you are using Microsoft Windows [Version 10.0.17763.2458]. As my searched results, native mdns was introduced in the version 10.0.18362.0 (1903/19H1) of Windows SDK.
  3. I also delete /WX /wd"4100" in CMakeLists.txt since it treat warning as error.

@sebastianhaberey
Copy link
Owner

That's good news! I updated the test environment to Windows Server 2022 (before, it was Windows Server 2019). There's only the choice between Windows Server 2022, 2019 and 2016.

It compiles now, but the tests don't seem to complete.

Do you have any ideas how to make them complete?

@sebastianhaberey
Copy link
Owner

I ran the tests again with verbose output. Still doesn't say much. I'll try if I can get more information about why the tests fail.

@lxp-git
Copy link

lxp-git commented Feb 12, 2022

Sorry, It is my fault. I will republish a new fix. IDE shows tests passed 6/6 tests, but its not.

@sebastianhaberey
Copy link
Owner

No worries! Let me know whenever you're ready - there's no hurry.

@sebastianhaberey
Copy link
Owner

Good news! I've finally released a proper Windows version of the plugin. It now passes all the integration tests, so it should have the same behavior as the other platform plugins. I would like to say a big thank you 💕 for writing your prototype. It was invaluable when I wrote the plugin, especially dealing with the immensely clunky Windows DNS API. If you hadn't implemented a great part of the functionality already, I'd probably have taken double the time to finish the plugin.

I've made a small "contributors" section in the readme, and you're the first major contributor 😊 Thanks again!

@lijy91
Copy link

lijy91 commented May 23, 2022

I think you should add @lxp-git as a contributor since nsd_windows is actually developed by him.

@sebastianhaberey
Copy link
Owner

Yes, fixed it, thank you!

@lxp-git
Copy link

lxp-git commented May 23, 2022

@sebastianhaberey I'd like to transfer nsd_windows of pub.dev to you if you need it.

@sebastianhaberey
Copy link
Owner

Hey thanks, do you know if that's possible? If yes, I'd be glad to use it.

@lxp-git
Copy link

lxp-git commented May 23, 2022

@sebastianhaberey I invite you as a uploader. Please accept the invitation. I will delete my self email if you ready. Maybe it will work.

@sebastianhaberey
Copy link
Owner

Nice, that actually worked - I could transfer the package to my own publisher. Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants