[capture, WaveRunner] Handling cycle based cfg for WaveRunner and debug #242
Conversation
johannheyszl
requested review from
andreaskurth,
nasahlpa,
vogelpi and
vrozic
as code owners
johannheyszl
force-pushed
the
waverunner_cleanups
branch
3 times, most recently
from
0d4a77c to
31f1a0c
Compare
There was a problem hiding this comment.
Thanks for your PR @johannheyszl ! I have some question regarding the WaveRunner setup.
| # TODO: Implement for Waverunner. Get sampling rate from scope and | ||
| # return. | ||
| return None | ||
| # Waverunner init not done yet, so cannot be read from WaveRunner. | ||
| raise RuntimeError("WAVERUNNER: ERROR: Sampling rate for WaveRunner " | ||
| "not given in configuration.") |
There was a problem hiding this comment.
Would there be an option to first init the scope with some example values, then read the configured sampling rate and do the config again with the proper values? This may sound very awkward but see my next comment for the motivation.
There was a problem hiding this comment.
The main issue is we want to convert cycles to samples before we create the scope because we hand those parameters to the constructor. But for the same reason we dont have a scope yet to read the sampling rate from it :)
| # Get current sampling rate from scope compare to cfg for sanity | ||
| setup_data = scope._ask("PANEL_SETUP?") | ||
| scope._get_and_print_cmd_error() | ||
| tmp_sampling_rate = int(re.findall(r'SampleRate = \d+', setup_data)[0][13:]) | ||
| if self.scope_cfg.sampling_rate is not None: | ||
| if self.scope_cfg.sampling_rate != tmp_sampling_rate: | ||
| raise RuntimeError("WAVERUNNER: Error: WaveRunner sampling " | ||
| "rate does not match given configuration!") |
There was a problem hiding this comment.
I think this is really non-ideal in the sense that if you want to change the sampling rate, you now have to configure the proper value on the scope and then also in the config file. So the user has to change the same value in two positions. I thought the whole reason for reading out the sampling rate value from the scope was to easily tune it without having to modify the config file. Instead of simplifying working with the framework, we now make it more difficult.
I suggest to do either:
- Removing the option to read out the sampling rate from the scope and make the framework more stupid but simpler.
- If a sampling rate is defined in the config, use that. If not, do an init of the WaveRunner scope before knowing the correct sample values, then read the sampling rate from the scope, compute the sample values and reconfigure the scope.
Do you think that would work?
There was a problem hiding this comment.
yeah it's not nice for the reason I stated in he above comment.
I am fine with different ways to deal with it:
- We could have two alternative scope constructors on the 'API', one based on samples, one based on cycles and internally they can nicely share the code.
- We can also remove the cycle cfg for waverunner alltogether.
- others ...
The only one I am opposed to is setting the sampling rate through config and loading it to the scope because it will mess up with the other settings very often and unpleasantly surprise lab technicians :D
…ng after scripting changes Signed-off-by: Johann Heyszl <[email protected]>
johannheyszl
force-pushed
the
waverunner_cleanups
branch
from
31f1a0c to
7e178ea
Compare
After scripting modifications and restructure re cycle based scope config for Husky, this PR now makes the modifications for WaveRunner to handle reasonably. Sampling rate is also given through cfg for cycle to sample converts, but then also fetched from scope and compared.
Also, some debug was required and fixes added.
Closes #210