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

IP addresses change does not trigger update in browser #235

Closed
PhilippSelenium opened this issue Mar 23, 2020 · 11 comments
Closed

IP addresses change does not trigger update in browser #235

PhilippSelenium opened this issue Mar 23, 2020 · 11 comments
Assignees

Comments

@PhilippSelenium
Copy link

Steps to reproduce

  1. Use example code from README:
from zeroconf import ServiceBrowser, Zeroconf


class MyListener:

    def remove_service(self, zeroconf, type, name):
        print("Service %s removed" % (name,))

    def add_service(self, zeroconf, type, name):
        info = zeroconf.get_service_info(type, name)
        print("Service %s added, service info: %s" % (name, info))

    def update_service(self, zeroconf, type, name):
        info = zeroconf.get_service_info(type, name)
        print("Service %s updated, service info: %s" % (name, info))

zeroconf = Zeroconf()
listener = MyListener()
browser = ServiceBrowser(zeroconf, "_http._tcp.local.", listener)
try:
    input("Press enter to exit...\n\n")
finally:
    zeroconf.close()
  1. Change IP address
    exp: An update message is printed.
    act: Nothing is printed

Notes:

  1. I can see the packets sent in wireshark.
  2. It works with version 0.24.4

zeroconf version: 0.24.5
python version: 3.6.7

@jstasiak
Copy link
Collaborator

jstasiak commented Apr 3, 2020

I narrowed this down to #228, cc @mattsaxon, I'll try to see if there's something obvious that I can fix quickly.

@jstasiak
Copy link
Collaborator

jstasiak commented Apr 3, 2020

I think this is because ServiceBrowser.update_record() only triggers ServiceStateChange on PTR and TXT record updates: https://github.com/jstasiak/python-zeroconf/blob/a79015e7c4bdc843d97bd5c82ef8ed4eeae01a34/zeroconf/__init__.py#L1428

@mattsaxon
Copy link
Collaborator

It’s is sort of by design that it doesn’t change with a changed IP address. We do need to change it so it triggers on SRV update though so I do appreciate this is both a regression and needs fixing, but I’d rather forward fix it to trigger on the SRV change as you suggest.

@jstasiak
Copy link
Collaborator

jstasiak commented Apr 3, 2020

Just so I understand – is it by design in relation to any part of RFC 6762 or 6763?

@mattsaxon
Copy link
Collaborator

Sorry, no I meant that the code was never designed to deal with an IP address change. The older code just got ‘lucky’, presumably
because a spurious txt update came through. We should explicitly recognise SRV changes though, so that is the right change to make.

@mattsaxon
Copy link
Collaborator

Assign this and the other regression to
me and I’ll sort them out when I get a chance

@emontnemery
Copy link
Collaborator

emontnemery commented Apr 6, 2020

Since this is a regression from #228, can #228 be reverted until IP address change is handled correctly?

@mattsaxon
Copy link
Collaborator

I should have forward fix available by end of today

@mattsaxon
Copy link
Collaborator

mattsaxon commented Apr 6, 2020

There is a possibility that my fix here is a breaking change as UpdateService used to get called on service addition. I feel this is wrong and was a mistake.

@jstasiak
Copy link
Collaborator

jstasiak commented Apr 6, 2020

Yeah I noticed that too, I agree this seems wrong and I'd say let's fix this.

@jstasiak
Copy link
Collaborator

Fix released in version 0.26.0.

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 a pull request may close this issue.

4 participants