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

Remove default URL to allow usage of a custom host name #35

Closed
wants to merge 2 commits into from

Conversation

eengnr
Copy link
Contributor

@eengnr eengnr commented Feb 24, 2024

In my LAN I use a custom DNS for local services which end with .home. Using http://winderoo.home was not possible, because the API was only available via http://winderoo.local resulting in a CORS error in the browser.
I removed the default URL from the API service which most likely is necessary to implement #8, too.
http://winderoo.local is still available, because it's set by the mDNS on the server side.
And this should also fix potential issues with trailing / for LAN configurations which do not start with 192.

@mwood77
Copy link
Owner

mwood77 commented Feb 25, 2024

Thanks for the contribution, again!

Couple points - we have to be careful here, as this change affects a number of things.

  1. The 192 href check is in place to see if the FE is running in development mode (FE is served from localhost:4200). The implementation in this PR breaks that functionality.

  2. Implementing changes around custom DNS forwarding is somewhat tricky, and we don't want to implement something that locks ourselves into a particular setup. Can you provide more information on how you handle your DNS re-direct from .local to .home? When deployed (built the filesystem image and flashed to a device), the FE is served from the webserver at winderoo.local as you've said. How are you serving the FE from a .home domain, and getting CORS errors? The webserver responds with the header <"Access-Control-Allow-Origin", "*"> which should be enough to combat most CORS issues. Are you sure it isn't your browser's "Referrer Policy" blocking your request, or the pre-flight requests getting stopped?

  3. Regarding multi-winder setups (Add support for multiple powered winders on one network #8), I've got some ideas on what to do here. I think the most reliable way to do this, would be to have the winders recognize 'new joiners' and configure themselves into a winder-mesh. From there, they can self-appoint a leader which provides the FE and coordinates subordinate winders. This should allow the winder-mesh to be flexible enough to allow devices to jump on/off the network - re-designating a leader each time - without a significant disruption to the user's experience. I've been putting this off for a reason 😅

I've refactored the ApiService file to be simpler, which removes the entire window href check entirely. You can now set the API url dynamically using Angular's environment variable substitution mechanism. Check out this draft PR -> #36

If you have an ESP32 handy, pull that branch, build the filesystem, and flash your board. I've included an updated build in the branch already.

@eengnr
Copy link
Contributor Author

eengnr commented Feb 25, 2024

To 1.)
Sorry, I didn't see that this was necessary!

To 2.)
You're right, this was probably a wrong setting in Firefox. I tried it with a new profile and now it works! (Tried with your PR #36)

To 3.)
That sounds like a very smart approach!

Sorry for the confusion, this PR can then be obviously closed :)

@eengnr eengnr closed this Feb 25, 2024
@mwood77
Copy link
Owner

mwood77 commented Feb 25, 2024

No worries, and there was no need to close your PR. I opened mine as an example, but I'm also happy to hear it worked for you! I'll merge it into master.

Please continue to bring up ideas, PRs, and issues!

@eengnr eengnr deleted the custom-host-name branch February 25, 2024 19:28
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.

2 participants