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

Proposal for dynamic rate limiting on SCPI communication #903

Open
chille opened this issue Sep 30, 2024 · 3 comments
Open

Proposal for dynamic rate limiting on SCPI communication #903

chille opened this issue Sep 30, 2024 · 3 comments
Labels
api API improvements core

Comments

@chille
Copy link
Contributor

chille commented Sep 30, 2024

The problem

When writing the driver for the OWON HDS200 I encountered a problem that had no perfect solution.

Some commands, like :DMM:MEAS?, can be sent without any rate limiting. When requesting data with SCPITransport::SendCommandQueuedWithReply() the call will be blocked until the data is returned. It is then safe to immidiately sent another command.

Other commands, like :DMM:RANGE {V, mV} takes a really long time. As the command is write only, there is no waiting for a reply. If a request is sent directly after, then it will either get dropped, or might possible get the wrong data. I currently block for around 800 ms to avoid this.

This problem have a few possible solutions:

  1. Do all the rate limiting with usleep(). This might freeze the GUI for a couple of seconds when doing things that takes longer time.

  2. Use the already built in rate limiting. Then everything will be slow. A :DMM:MEAS? will do something like 1.2 readings per second, instead of 15. Other things like the scope might be even worse.

  3. Extend the SCPITransport to have some kind of dynamic rate limiting. Something where each command could have a "settle time" that will block the next command until it is safe to run. This is the solution I propose.

How do we implement this?

API changes

All SCPITransform::Send*() functions will have an extra argument added:
std::chrono::milliseconds settle_time = chrono::milliseconds(0)
This settle_time is optional and will default to 0 ms.
This will not need any refactoring in other classes.

Storing the required data

SCPITransport::m_txQueue will be changed from:
std::list<std::string> m_txQueue;
to
std::pair<std::string,std::chrono::milliseconds> m_txQueue;

In this way we can store both the command and the associated "settle time".

Nothing in the scopehal-apps repo uses m_txQueue outside of the SCPITransport class. So this should be easy to change.

Sending data

All code that does some kind of timing in SCPITransport should do it like this instead:

If settle_time > 0
	Use settle_time
Else
	Use m_rateLimitingInterval

I have not yet looked into what exact stepts that will be needed for this. It could possibly be as easy as letting RateLimitingWait() be aware of the settle_time.

Notes

  • If settle_time > 0, then this value will always be used, even if it is lower than the global rate limit.
@azonenberg
Copy link
Collaborator

This looks like the right way to do this. Go ahead and throw together a prototype implementation and see how it works for your hardware?

The transport object model was designed to be pretty self-contained for exactly this reason, so it should be a fairly straightforward change.

@chille
Copy link
Contributor Author

chille commented Sep 30, 2024

Great! I will continue with the development and prepare a pull request when ready!

@azonenberg azonenberg added api API improvements core labels Sep 30, 2024
@chille
Copy link
Contributor Author

chille commented Oct 3, 2024

I have successfully implemented and tested the dynamic rate limiting API. I will continue the development of the OWON HDS200 driver to test it a bit more and then open a pull request.

While doing so I have found a few more things we need:

  1. Some way to make all calls always be blocking. Basically something that "auto flushes" the command queue immidiately. Otherwise this will cause a lot of extra work when writing your own application for automated testing linking with scopehal. If I dmm.SetMeterMode(Multimeter::DC_VOLTAGE); then I want to be sure that the command is sent immidiately and is blocking until it is safe to do a dmm.GetMeterValue();. I have solved this right now by using the SendCommandImmediate*() functions. But a queue with the option to enable auto flush might be a better idea, as it will be thread safe.

  2. We really need some way to be able to add an additional delay to commands. There is a lot of trickery going on in the OWON HDS200 driver right now. I can properly time the massages on my host, but a router in between me and the instrument might merge these two packets together before reaching the instrument. If this happens some things will break. This is of course impossible to avoid. But to have the option to "delay all packets an extra 1200 ms" might mitigate the problem enough. This should probably be in the SCPISocketTransport class. This will also give us the ability to "simulate a slow VPN" as debugging/development feature.

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

No branches or pull requests

2 participants