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

Implemented mDNS #1441

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Implemented mDNS #1441

merged 1 commit into from
Nov 15, 2023

Conversation

CommanderRedYT
Copy link
Contributor

@CommanderRedYT CommanderRedYT commented Oct 20, 2023

This PR implements mDNS for OpenDTU. It adds two services, one for the commonly used _http._tcp service, and one custom one (_opendtu._tcp) for auto-discovery.

It also implements the needed changes for the Web-App.

Flash Statistics:

$ du .pio/build/generic_esp32/firmware.bin

# before: 1512
# after: 1540

This PR was inspired by #695.

@CommanderRedYT
Copy link
Contributor Author

CommanderRedYT commented Oct 20, 2023

Note: I have commented out the deinit code for mDNS inside NetworkSettings.cpp. With it commented in, the ESP crashes inside lwip tcp functions. I am unfortunately not able to debug this in detail, so help is required there. Other option would be to maybe add a note that after deactivating mDNS, user has to restart the ESP.

Moved mDNS code out of request handler, could not reproduce crash anymore.

@CommanderRedYT
Copy link
Contributor Author

I cleaned it up a little bit more. I looked at some other projects that use esp32 and mDNS (like WLED for example), and they all only enable/disable mDNS on boot. This means that every time you'd change this option you would have to reboot.

I never had problems with it when turning it on, so if there are problems I could rewrite it so that enabling will work, but disabling only works on boot. But imo it works fine like this.

Copy link
Owner

@tbnobody tbnobody left a comment

Choose a reason for hiding this comment

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

Would you mind combining the commits into one with git squash?

include/defaults.h Outdated Show resolved Hide resolved
webapp_dist/index.html.gz Outdated Show resolved Hide resolved
webapp_dist/js/app.js.gz Outdated Show resolved Hide resolved
webapp_dist/zones.json.gz Outdated Show resolved Hide resolved
src/NetworkSettings.cpp Show resolved Hide resolved
src/NetworkSettings.cpp Outdated Show resolved Hide resolved
src/NetworkSettings.cpp Outdated Show resolved Hide resolved
@CommanderRedYT
Copy link
Contributor Author

@tbnobody Thanks for the Review, I have resolved all of the issues :)

Copy link
Owner

@tbnobody tbnobody left a comment

Choose a reason for hiding this comment

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

Sorry, just had to add some remarks (it triggered my "Internal Monk")

src/NetworkSettings.cpp Outdated Show resolved Hide resolved
src/NetworkSettings.cpp Outdated Show resolved Hide resolved
src/NetworkSettings.cpp Outdated Show resolved Hide resolved
@CommanderRedYT
Copy link
Contributor Author

Okay, fixed everything again 👍

@CommanderRedYT
Copy link
Contributor Author

Btw we should maybe link issue #1428 with this PR

@CommanderRedYT
Copy link
Contributor Author

@tbnobody Any updates?

@tbnobody
Copy link
Owner

tbnobody commented Nov 5, 2023

It's already included in my local branch for several days. Just had no time for a release yet. Will (hopefully) publish it tomorrow or tuesday.

@CommanderRedYT
Copy link
Contributor Author

It's already included in my local branch for several days. Just had no time for a release yet. Will (hopefully) publish it tomorrow or tuesday.

Awesome, thank you very much! Have a nice evening

@tbnobody tbnobody merged commit 89b4b8e into tbnobody:master Nov 15, 2023
Copy link

github-actions bot commented Apr 2, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2024
@CommanderRedYT CommanderRedYT deleted the opendtu-mdns branch October 5, 2024 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants