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

set the connected status of a socket after UDP & raw connect #9251

Merged
merged 1 commit into from
May 22, 2024

Conversation

jepler
Copy link
Member

@jepler jepler commented May 15, 2024

This is a speculative fix for adafruit/Adafruit_CircuitPython_NTP#33

@anecdata
Copy link
Member

anecdata commented May 15, 2024

NTP lib: (edit: v3.1.0, was convenient for testing)

                sock.connect(self._socket_address)
                sock.send(self._packet)
                sock.recv_into(self._packet)

This PR:
Adafruit CircuitPython 9.1.0-beta.1-25-g13ea06f1ae on 2024-05-15; Raspberry Pi Pico W with rp2040

code.py output:
struct_time(tm_year=2024, tm_mon=5, tm_mday=15, tm_hour=18, tm_min=41, tm_sec=14, tm_wday=2, tm_yday=136, tm_isdst=-1)
# etc...

Previously:
Adafruit CircuitPython 9.1.0-beta.1 on 2024-04-19; Raspberry Pi Pico W with rp2040

Traceback (most recent call last):
  File "code.py", line 53, in <module>
  File "adafruit_ntp.py", line 92, in datetime
BrokenPipeError: 32

@jepler
Copy link
Member Author

jepler commented May 15, 2024

thanks for the quick testing. I hadn't even gotten a board out yet.

@dhalbert
Copy link
Collaborator

The .send() was changed to sendto() just now in version 3.1.1 of the NTP library.

@justmobilize
Copy link

Is there any other testing that's needed? I could see this coming up somewhere else...

@anecdata
Copy link
Member

anecdata commented May 22, 2024

I didn't test RAW, only used the (previous, interim) NTP lib to test UDP. Not sure if anyone uses RAW.

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

I will go ahead and merge this. The RAW case seems like it should be the same. We can fix if there's something pathological.

@dhalbert dhalbert merged commit 3418992 into adafruit:main May 22, 2024
22 checks passed
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.

None yet

4 participants