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

Simplify timeouts and enable for both servers and clients by default #39

Merged
merged 4 commits into from
Oct 15, 2023

Conversation

TMRh20
Copy link
Member

@TMRh20 TMRh20 commented Oct 14, 2023

  • Add timeouts for both clients and servers by default
  • Change default timeout period to 45 seconds
  • Remove 4-bytes from userdata struct (connAbortTime)
  • Server-side timeouts were not working by default, now they are. Set UIP_CONNECTION_TIMEOUT in uip-conf.h to a higher value or 0 to disable

Was finding an issue I noticed in the past with the default behaviour of servers hanging up if they experienced a disconnect etc. The solution was to manually set a timeout, but it was not enabled by default. This sets it up by default to timeout of data is not received or acked in 45 seconds and simplifies the code.

- Add timeouts for both clients and servers by default
- Change default timeout period to 45 seconds
- Server-side timeouts were not working by default, now they are. Set to a higher value or 0 to disable
Copy link
Member

@2bndy5 2bndy5 left a comment

Choose a reason for hiding this comment

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

This does indeed simplify the code.

- Only set connection timer if userdata has been allocated
The initial connection logic should come first
@2bndy5
Copy link
Member

2bndy5 commented Oct 15, 2023

FYI, the Arduino CI does the clang-format check on this repo but only for changes to examples or changes to the .github/workflows/build_arduino.yml. It would be a good idea to run clang-format-12 on this branch before merging.

@TMRh20 TMRh20 merged commit 792b0be into master Oct 15, 2023
4 checks passed
@2bndy5
Copy link
Member

2bndy5 commented Oct 15, 2023

I probably should have pointed out that clang-format is told to ignore Dns.cpp and Dns.h files because they seem to be directly copied from the UIP library.

check_formatting:
uses: nRF24/.github/.github/workflows/cpp_lint.yaml@main
with:
ignore: 'utility|clock-arch.c|clock-arch.h|Dns.cpp|Dns.h'
extensions: ino,cpp,h

While I like the improved readability, those file changes could be reverted in the future if desired (or if the Apache License v2.0 stipulates that any changes be explicitly noted).

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