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

Configurable polling interval? #41

Closed
tycho opened this issue May 18, 2017 · 9 comments
Closed

Configurable polling interval? #41

tycho opened this issue May 18, 2017 · 9 comments
Assignees

Comments

@tycho
Copy link

tycho commented May 18, 2017

Can you please make the card monitoring thread's polling interval configurable?

My current hack works, but makes me sad:

        self._monitor = CardMonitor()
        self._monitor.addObserver(self)

        success = False
        attempts = 0
        sleeptime = 0.01
        while attempts < 5:
            try:
                self._monitor.rmthread.cardrequest.pcsccardrequest.pollinginterval = 5
                success = True
                break
            except:
                time.sleep(sleeptime)
                sleeptime = 0.1
            attempts += 1
        if success:
            print("Successfully set polling interval")
        else:
            print("Failed to set polling interval")

CardMonitoringThread's cardrequest field doesn't exist until the thread starts. So I have to wait before I can monkey-patch the polling interval value.

It'd be infinitely better if I could just set the polling interval when I create a CardMonitor instance, or if I could use a function call to set it (or both!)

@LudovicRousseau
Copy link
Owner

Active polling is bad and should not be used by PySCard. SCardGetStatusChange() should be used instead.

I do not use PCSCCardRequest() and I am not the author of this code.

I will have a look at the code. But I don't know when I will have time for that.
So a patch to avoid polling is really welcome :-)

@LudovicRousseau
Copy link
Owner

I had a look at the waitforcard() and waitforcardevent() code
https://github.com/LudovicRousseau/pyscard/blob/master/smartcard/pcsc/PCSCCardRequest.py#L108
The code is very complex.

My problem is that I am not sure of what the methods are supposed to do. The documentation is very limited.
For example how to manage reader insertion or removal? Should the method just ignore readers added after the method started?

@tycho, why do you want to change the polling interval?

@tycho
Copy link
Author

tycho commented May 19, 2017

A fair question.

The app I'm using the library in has an option to minimize to the system tray. And while it's there, it doesn't need to interact with a smart card. The problem is that the pyscard ReaderMonitor and CardMonitor threads poll so frequently that the app will do nothing useful and eat away at battery life on laptops.

My first option was to clean up everything pyscard related and re-initialize it all when the app is restored from the tray. But then I started running into all kinds of weird crashy behavior that I couldn't explain. It turns out it's really hard to properly shut down and clean up all the pyscard stuff.

The second option is to leave the pyscard objects as they are, but make them poll less frequently to eat less CPU. So when my app is minimized to the tray, I set a really long poll interval, and when the app is restored, I restore the original short poll intervals.

@LudovicRousseau
Copy link
Owner

I do not plan/have time to rewrite ReaderMonitor and CardMonitor now.
So a patch is welcome.

Maybe you can change your application to stop using ReaderMonitor and CardMonitor while in the system tray?

@tycho
Copy link
Author

tycho commented May 23, 2017 via email

@ElouanPetereau
Copy link
Contributor

Hello,

I also add troubles with the threads polling speed while testing pyscard.
I was using the PC/SC implementation for linux (pcsc-lite) and because of the fast polling speed, the pcsc daemon logs were completely filled with these calls making it unreadable.

After giving a look at the CardMonitor and ReaderMonitor code, I discovered that you can set the polling interval speed for the ReaderMonitor but not for the CardMonitor.

There is also a pollinginterval parameter for the cardrequest.pcsccardrequest object (this is the one that tycho was monkey-patching).
This one is used by the cardrequest.waitforcard() and cardrequest.waitforcardevent() methods which call SCardGetStatusChanged() and loop using the pollinginterval only if an error is detected and if the timeout has not been reached. But because the sleep is done before checking this, the call will be at least cardrequest.pcsccardrequest.pollinginterval long.

The current CardMonitor implementation create cardrequest.pcsccardrequest object and loop with the waitforcardevent() method with a timeout of 0.1 seconds until a a specific call to stop the monitoring thread is done.
Because the cardrequest.pcsccardrequest polling interval is 0.1 seconds and because in the waitforcardevent() methods the sleep is executed, then the call to SCardGetStatusChanged(), then the check for the end of the timeout, only one call is done thus changing cardrequest.pcsccardrequest.pollinginterval before calling waitforcardevent work.
But the timeout have to be at equal or inferior to the pollinginterval parameter !

So, I think, a polling interval between each waitforcardevent() call inside the CardMonitor could be added to the object parameters to simplify its use.
This polling interval will then be set to both the timeout and the polling interval of the cardrequest.pcsccardrequest object before calling waitforcardevent().

On top of that, I also discovered that when you create a CardMonitor object, a card monitoring thread is automatically created and thus polling calls are made even if no observer have been added to the monitor.
I don't think this is the desired beahvior because the comments on the CardMonitoring.py file explain this:

note: a card monitoring thread will be running
as long as the card monitor has observers, or CardMonitor.stop()
is called. Do not forget to delete all your observers by
calling deleteObserver, or your program will run forever...

This is due to the _START_ON_DEMAND_ variable in CardMonitoring.py being set to False by default.
In Readermonitoring.py, I saw that the variable that as the same role (startOnDemand) is set to True by default which is, from what I understood, the required behaviour.

I made some changes to CardMonitor that makes it respect the behaviour described above and that allow you to edit the polling interval speed.I also decided to use a Borg Singleton implementation for the Cardmonitoring, like it is done for Readermonitoring, instead of the current "classic" Singleton implementation.

If you are interested in these changes, or if you want to add them to pyscard @LudovicRousseau, they are available on my fork of pyscard on the refactor_cardmonitor branch.

@LudovicRousseau
Copy link
Owner

I did not write the CardMonitor and ReaderMonitor code.
Using an active loop with a low timeout value is a bad idea.
On Windows and GNU/Linux we could use the special reader "\\?PnP?\Notification" to detect reader events. But the problem is that macOS does not support "\\?PnP?\Notification".

Thanks for the patches @ElouanPetereau. I may have a look but I don't know when.

@ElouanPetereau
Copy link
Contributor

You're welcome.

And yes of course using CardMonitor and ReaderMonitor is not the efficient and cpu/battery friendly way to go. I only made these patches to reduce the problem.

@LudovicRousseau
Copy link
Owner

This is now fixed in version 2.2.0.
Yeah!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants