Feature suggestion for Waveform view: Analysis tool default signal based on mouse position when right-clicking

Future Features
1

When using the analysis tools it looks like the default channel is always the current. I think the mouse position could be used along with information about the channel y boundaries within the GUI to select between the current, voltage, etc. which I think would be more intuitive and a bit faster for users.
A separate but adjacent feature would be to have all available channels in the signal drop down, even if they’re not currently displayed. E.g. It might be useful to do frequency analysis of the voltage even if only the current is displayed.

2

Hi @Jeremy - I think that the Waveform widget in UI 1.x supports this, but we never updated the range tools to take advantage of this information. See _on_range_tool, which parses the mouse location to determine the default. Here is the value passed to the range tool’s !run action when I click on the voltage plot:

{
  "x_range": [
    250711945197826646,
    250711946153704714
  ],
  "origin": "WaveformWidget:00000001",
  "signals": [
    "JsdrvStreamBuffer:001.JS220-002557.i",
    "JsdrvStreamBuffer:001.JS220-002557.v"
  ],
  "quantity": "v",
  "signal_default": "JsdrvStreamBuffer:001.JS220-002557.v"
}

Note quantity and signal_default.

Your custom range tool(s) as discussed on “Is it possible to add quantities that are calculated from the data to the waveform summary and dual markers?” can use this info to “do the right thing”.

A range tool is free to ignore any or all of this information and pull it’s own. For the range tools we have, I don’t see any value in using signals not being displayed in the Waveform widget.

3

Hi Matt, Thanks!

I plan to change the following code in the .*ToolDialog __init__ based on cdf.py going from:

for signal_id in value['signals']:
            signal_name = '.'.join(signal_id.split('.')[-2:])
            self._signal.addItem(signal_name)

to:

        default_idx = 0
        for idx, signal_id in enumerate(value['signals']):
            if signal_id == value['signal_default']:
                default_idx = idx
            signal_name = '.'.join(signal_id.split('.')[-2:])
            self._signal.addItem(signal_name)
        self._signal.setCurrentIndex(default_idx)

The implementation in histogram.py already had a for … enumerate, but I listed the cdf.py code in case anyone else wants to modify the same areas in the other range tools.

Should I try to submit a pull request for the existing range tools with the change?
I have tested it out only briefly with each tool, but it seems to work and the pylint scores don’t go down. I don’t think this is urgent at all since it’s only for convenience.

4

Hi @Jeremy - I’d be happy to accept a pull request with this improvement. I recommend refactoring this feature into a function. Perhaps
joulescope_ui.range_tools.plugin_helpers.signal_combobox_config like this:

def signal_combobox_config(combobox: QtWidgets.QComboBox, value):
    default_idx = 0
    for idx, signal_id in enumerate(value['signals']):
        if signal_id == value['signal_default']:
            default_idx = idx
        signal_name = '.'.join(signal_id.split('.')[-2:])
        combobox.addItem(signal_name)
    combobox.setCurrentIndex(default_idx)
5

Hi Matt,
I think consolidating the common code to a function is a good idea and have implemented that code in the suggested plugin_helpers file.
The other range tools have been updated with the new function, and an item at the top of the CHANGELOG.md added/updated (based on the instructions in the CONTRIBUTING.md).
I didn’t initially fork the repo, so I’ve forked it and re-remoted my clone. I’ve pushed the changes on a branch “feature/317” and created a change request for it.

6

Thanks for the PR! Merged and will be in the next Joulescope UI release!