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

Add usage of select instead of pulling for Linux based platform (PCAN Interface) #1410

Conversation

BKaDamien
Copy link
Contributor

Avoid pulling of "time.sleep(0.001)" in pcan interface method _recv_internal, and use select python function instead.

@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #1410 (b40b391) into develop (4b1acde) will decrease coverage by 0.53%.
The diff coverage is 81.39%.

❗ Current head b40b391 differs from pull request most recent head b0796b8. Consider uploading reports for the commit b0796b8 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1410      +/-   ##
===========================================
- Coverage    65.16%   64.64%   -0.53%     
===========================================
  Files           81       81              
  Lines         8853     9183     +330     
===========================================
+ Hits          5769     5936     +167     
- Misses        3084     3247     +163     

@felixdivo
Copy link
Collaborator

Looks good! I haven't tested it with hardware though. We should probably do that.

@BKaDamien
Copy link
Contributor Author

Looks good! I haven't tested it with hardware though. We should probably do that.

Tested on my side, this update will be a little less consumer than the polling.

@felixdivo
Copy link
Collaborator

Sorry for being this pedantic, but on which platforms did you try this? There appear to be 4 variants here: Windows and Linux and each with and without polling. For each of the 4, sending and receiving has to work. 🙂

@BKaDamien
Copy link
Contributor Author

Sorry for being this pedantic, but on which platforms did you try this? There appear to be 4 variants here: Windows and Linux and each with and without polling. For each of the 4, sending and receiving has to work. 🙂

Based on my changes:
Tested on Windows -> OK (use WaitForSingleObject windows base event)
Test on Ubuntu -> OK (use python select base event)
For the other MAC, .... -> Just use the polling

@felixdivo
Copy link
Collaborator

felixdivo commented Nov 14, 2022

Okay, thank you for documenting that here. 🙂 I think this is sufficient, since the polling implementation did not really change and there are also unit tests that make sure nothing gets too broken.

I'd propose to merge it as it.

@BKaDamien
Copy link
Contributor Author

BKaDamien commented Nov 14, 2022

Okay, thank you for documenting that here. 🙂 I think this is sufficient, since the polling implementation id not really change and there are also unit tests that make sure nothing gets too broken.

I'd propose to merge it as it.

You are the boss :)

@BKaDamien BKaDamien closed this Nov 14, 2022
@BKaDamien BKaDamien reopened this Nov 14, 2022
@BKaDamien
Copy link
Contributor Author

Arf I wrongly click and close I am sorry

@felixdivo
Copy link
Collaborator

@hardbyte You can merge it before or after the 4.1.0 release. If you add it before, we'd have to amend the changelog.

@zariiii9003 zariiii9003 force-pushed the feature/add_event_based_reception_for_pcan_interface_on_linux branch from b40b391 to 890eee3 Compare December 21, 2022 00:27
@zariiii9003
Copy link
Collaborator

@BKaDamien I cleaned up a bit, please take a look.

@BKaDamien
Copy link
Contributor Author

@BKaDamien I cleaned up a bit, please take a look.

It's fine. Thanks for the chnages.

@zariiii9003 zariiii9003 merged commit 5c523ec into hardbyte:develop Jan 23, 2023
Tbruno25 pushed a commit to Tbruno25/python-can that referenced this pull request Jan 25, 2023
… Interface) (hardbyte#1410)

* - integrate handling of Linux based event using select for pcan interface

* - adapt pcan unit tests regarding PCAN_RECEIVE_EVENT GetValue call

* - add the case HAS_EVENTS = FALSE whne no special event basic mechanism is importes

* - force select mock for test_recv_no_message

* - just for testing : see what is going wrong with CI testing and my setup

* - correct patch on pcan test test_recv_no_message

- run black

* - remove debug log

* -just reformat test_pcan

* - move global variable IS_WINDOW and IS_LINUX from pcan to basic module

* - remove redundant comment in pcan _recv_internal

* refactor _recv_internal

Co-authored-by: zariiii9003 <52598363+zariiii9003@users.noreply.github.com>
@cody82 cody82 mentioned this pull request Mar 22, 2023
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.

None yet

3 participants