-
Notifications
You must be signed in to change notification settings - Fork 11
Fixed error on Python3 #7
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for that. It seems that there is interest in supporting CPython so I'm happy to consider this. Let me run some tests on MicroPython to see how much difference it makes to the bytecode size in practice. |
Faked handling of unicast replies seems to cause replies not reaching its destination in some cases
Used for standalone execution
slimDNS.py
Outdated
|
|
||
| if __name__ == "__main__": | ||
| if len(sys.argv) < 3: | ||
| print("Usage: {} local_addr local_host".format(sys.argv[0])) | ||
| sys.exit(1) | ||
| local_addr = sys.argv[1] | ||
| local_host = sys.argv[2] | ||
| server = SlimDNSServer(local_addr, local_host) | ||
| server.run_forever() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to have a command-line wrapper for the library in a separate file but in the interests of size I'd like to keep this out of the main code.
slimDNS.py
Outdated
|
|
||
| # XXX Always multicast the reply. Faked handling of unicast replies | ||
| # seems to cause replies not reaching its destination in some cases. | ||
| self.sock.sendto(buf[:o], (_MDNS_ADDR, _MDNS_PORT)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I'm sort of surprised that the old code worked at all, since it looks like I was comparing the source IP address to _MDNS_PORT. Can you try:
self.sock.sendto(buf[:o], (_MDNS_ADDR, _MDNS_PORT) if addr[1] == _MDNS_PORT else addr)
and see if it fixes the problem you were having?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing addr[0] to addr[1] as you suggested fixes the problem I am having.
| ticks_ms = lambda : time.clock() * 1000. | ||
| ticks_diff = lambda a, b: b - a | ||
| else: | ||
| ticks_ms = time.ticks_ms | ||
| ticks_diff = time.ticks_diff | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. This does not seem like undue overhead to support CPython.
_MDNS_PORT was wrongly compared with source IP address
|
I have separated the command line wrapper and I have tested that the fix for unicast reply is working correctly. Do you have any other input on this pull request? |
|
On second thought, does it really make sense from your point of view to accept this pull request if it contains the command line wrapper? If not, as long as the fix for python 3 & the unicast reply is accepted, I can just revert the command line wrapper and create my own project which depend on this project and contains the command line wrapper only. |
|
I think that the command line wrapper in a separate file is just fine here. When used in a MicroPython setting it will typically get copied onto the microcontroller manually anyway. |
I know that in #4 and #5 this proposal by @nevercast has been rejected because we want as small footprint as possible for MicroPython.
However, my use case is to use this as lightweight replacement for avahi-daemon in Raspbian on Raspberry Pi.
This library can also be potentially useful for enhancing https://github.com/dapperfu/python_mDNServer so that it does not depend on avahi-daemon anymore.
I guess I will just leave this pull request here for anyone who is interested.