Skip to content

Replaced select.select() with select.epoll() in SocketcanBus - #2056

Merged
zariiii9003 merged 3 commits into
hardbyte:mainfrom
bhicks-stackav:socketcan-epoll
Nov 11, 2026
Merged

Replaced select.select() with select.epoll() in SocketcanBus#2056
zariiii9003 merged 3 commits into
hardbyte:mainfrom
bhicks-stackav:socketcan-epoll

Conversation

@bhicks-stackav

@bhicks-stackav bhicks-stackav commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

  • Replace usage of select.select() with select.epoll() in SocketcanBus to avoid a ValueError being raised when a high number of files (>1024) are open

Related Issues / Pull Requests

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring
  • Other (please describe):

Checklist

  • I have followed the contribution guide.
  • I have added or updated tests as appropriate.
  • I have added or updated documentation as appropriate.
  • I have added a news fragment for towncrier.
  • All checks and tests pass (tox).

Additional Notes

@zariiii9003

zariiii9003 commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Your solution is better than #2057, if you can make the tests pass i will merge this one.

@mergify

mergify Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@bhicks-stackav

bhicks-stackav commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@zariiii9003 sorry about that. I believe I've addressed the issue. Please take another look.

@bhicks-stackav

bhicks-stackav commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@Mergifyio queue

@mergify

mergify Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

queue

☑️ Command disallowed due to command restrictions in the Mergify configuration.

Details
  • sender-permission >= write

@bhicks-stackav

bhicks-stackav commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@zariiii9003 friendly ping on this. Please take another look when you get a chance.

@bhicks-stackav

bhicks-stackav commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@zariiii9003 it seemed like the most recent test failures were due to package/env issues unrelated to this change (e.g., brew not allowing coveralls on macos tests). I just rebased the branch. Could you please run the CI tests one more time?

@bhicks-stackav

bhicks-stackav commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@zariiii9003 ping on this

@bhicks-stackav

bhicks-stackav commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

@zariiii9003 ping on this again. Could you please run the CI tests one more time?

@TheTrueAI TheTrueAI mentioned this pull request Oct 25, 2026
Merged
3 of 10 tasks
zariiii9003
zariiii9003 approved these changes Nov 11, 2026
View reviewed changes

@zariiii9003 zariiii9003 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you and sorry for the small delay 👀

@zariiii9003
zariiii9003 merged commit 4930b53 into hardbyte:main Nov 11, 2026
28 checks passed
This was referenced Nov 11, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zariiii9003 zariiii9003 zariiii9003 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SocketcanBus does not support socket file descriptors over 1023

2 participants

@bhicks-stackav @zariiii9003