-
Notifications
You must be signed in to change notification settings - Fork 3
Fix PicoScope streaming issues #188
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
Conversation
drslebedev
commented
Dec 18, 2025
- Avoid duplicate digitalOutSpan.publishTag() when samples are dropped
- Increase internal driver buffer size to gain performance for high sample rate
- Log actual sample rate in verbose mode
- Fix TimingMatcher alignment for negative deltas (std::floor) and use a single safeSamples threshold
d3066f7 to
996440c
Compare
| return std::unexpected{Error{"No channels configured"}}; | ||
| } | ||
| scope.data.resize(65536); // todo: figure out better buffer size strategy | ||
| scope.data.resize(16 * 65536); // todo: figure out better buffer size strategy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't hard-code this number; fix it, e.g., by making it depend on the sampling and polling rates, a configurable integer parameter, and a configurable minimum size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! The buffer size now depends on the sample rate and the number of active channels. Polling rate isn’t considered yet, because the user can’t configure it, and the PicoScope block’s effective polling rate is simply the scheduler’s processBulk execution rate.
const std::size_t kBaseBuf = 16384;
const std::size_t mult = std::max<std::size_t>(1, static_cast<std::size_t>(std::ceil(freq / 1e6)));
scope.data.resize(activeChannels * mult * kBaseBuf);I didn’t expose the base buffer size as a user setting, because it is not intuitive and depends on the formula we use (and that formula may change in the future). What would likely be more intuitive for the user is the ability to configure the polling rate, but that isn’t supported right now.
| actualFreq = detail::convertTimeIntervalToSampleRate(timeInterval); | ||
| started = true; | ||
| if (scope.verbose) { | ||
| std::println("actualFreq:{}, timeInterval:{}, units:{}", actualFreq, timeInterval.interval, magic_enum::enum_name(timeInterval.unit)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This print-out will kill any performance metric for polling rates > 100 Hz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message is printed only once at startup, so it shouldn’t affect performance. It’s also only printed when console_verbose is set to true.
- Avoid duplicate digitalOutSpan.publishTag() when samples are dropped - Increase internal driver buffer size to gain performance for high sample rate - Log actual sample rate in verbose mode - Fix TimingMatcher alignment for negative deltas (std::floor) and use a single safeSamples threshold Signed-off-by: drslebedev <dr.s.lebedev@gmail.com>
996440c to
506d967
Compare
|


