Skip to content

Fix fileno error on Windows (robotell bus) #1313

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

Merged
merged 4 commits into from
May 25, 2022

Conversation

gribera
Copy link
Contributor

@gribera gribera commented May 20, 2022

Handling io.UnsupportedOperation and raising NotImplementedError prevents #1312 bug

@codecov
Copy link

codecov bot commented May 21, 2022

Codecov Report

Merging #1313 (7c034a4) into develop (796b525) will decrease coverage by 0.02%.
The diff coverage is 60.00%.

@@             Coverage Diff             @@
##           develop    #1313      +/-   ##
===========================================
- Coverage    66.00%   65.98%   -0.03%     
===========================================
  Files           86       86              
  Lines         8945     8948       +3     
===========================================
  Hits          5904     5904              
- Misses        3041     3044       +3     

@zariiii9003
Copy link
Collaborator

Can you add a test that ensures a NotImplementedError on Windows but not on Linux?

@gribera
Copy link
Contributor Author

gribera commented May 23, 2022

I added a function that test the fileno method, but in the tests will always raise a NotImplementedError in both Windows and Linux because it is opening the serial port as 'loop://', which simulates a loopback connection, so I think it is not necessary to discriminate the OS

The bad thing is that I don't know how to test if the function returns a valid fileno

@zariiii9003 zariiii9003 added this to the 4.0.1 milestone May 25, 2022
@zariiii9003 zariiii9003 merged commit 4cb2f2f into hardbyte:develop May 25, 2022
@zariiii9003
Copy link
Collaborator

Thank you 👍

MattWoodhead added a commit to MattWoodhead/python-can that referenced this pull request Jun 19, 2022
Change implemented similar to that made in hardbyte#1313. This means notifiers will now work with the Serial interface.
zariiii9003 pushed a commit that referenced this pull request Jun 22, 2022
* Add conda-forge badge to readme

* Update fileno method in serial_can.py

Change implemented similar to that made in #1313. This means notifiers will now work with the Serial interface.

* Format serial_test.py with black

* Revert "Add conda-forge badge to readme"

This reverts commit 59d0b3c.
@felixdivo felixdivo modified the milestones: 4.0.1, 4.1.0 Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants