Signal handles and multi-axis support#9
Signal handles and multi-axis support#9TimonTerBraak wants to merge 10 commits intowindelbouwman:masterfrom
Conversation
|
Glad to see this change was possible at all, I think the codebase can be modified fairly easy. I'm not too sure about this change, one thought I had is how far into the direction of logic analyzer / scope I would like to go. There are good tools such as sigrok which are logic analyzers which usually have this kind of view. Another idea about the multi axis would be to have an option for the plot to show a seperate axis for each signal in the plot, and allow to select it. Then the vertical zooming/panning will be done for the selected signal. I think at least all axis should be visible, preferably with the color of the signal. Maybe we also require the grid to be present multiple times? Anyway, good proposal! I'm not sure where this whole thing is going, so I would like to keep this idea around for some time. |
| self._draw_curves() | ||
| self._draw_legend() | ||
|
|
||
| if self.options.show_legend: |
There was a problem hiding this comment.
This option certainly makes sense.
| if data: | ||
| if isinstance(data[0], Aggregation): | ||
| self._draw_aggregations_as_shape(data, curve_color) | ||
| curve.average = self._draw_aggregations_as_shape(curve.axis, data, curve_color) |
There was a problem hiding this comment.
You could query the average from by querying the summary of the data in view to the time series database (tsdb). This will give you a quicker result than re-calculating the average in the draw function.
Also: having the draw function doing two things (draw the plot and calculate the average) is probably not so clean, it would be better to seperate the two functions.
There was a problem hiding this comment.
Sure, still have to get familiar with the codebase.
| self.chart.horizontal_zoom(amount, around) | ||
| # Autoscale Y for a nice effect? | ||
| self.chart.autoscale_y() | ||
| self.repaint() |
There was a problem hiding this comment.
What does this function do? update also triggers a paint action eventually. Does this result in snappier look / feel?
There was a problem hiding this comment.
The handles on the left hand side otherwise lagged behind in the repainting. Not sure about the reason though. Maybe that 'update' only invalidated the chart area itself?
Yes, it is quite accessible.
The problem of plotting multiple signals with a vast different range remains. If one signal is of orders of magnitude larger than a second one, plotting them in the same area makes little sense for the secondary signal. This is typically resolved by scaling the range of either of the signals (using a dedicated axis).
Note that in the 'legend bar' branch, the selectability of signals has improved and it is always clear what signal is selected. Just to let you know... The challenge here is to keep the plot area as clean as possible. Not even all decent scopes are good at that. The visuals of the data aggregations are nice, but are at the same time already much more that a single line. Adding multiple grids and axis gives too much clutter, perhaps. |
9d0c231 to
63233b2
Compare
Maybe you can have a look at this? Not necessarily merge it right away, it probably needs some work / thinking, even if you would like this feature.
This branch prototyped multi-axis support, such that signals can individually scaled and panned, as it is quite common that correlated signals have a vast different domain. It also adds 'signal handles' to the left of the plot, similar to most digital oscilloscopes. This handle can be used for both panning and selecting that signal as being 'active'. Note that the vertical axis on the right is then changed to the active signal. While correct, this part needs some visual changes to ensure that it is clear to which signal the vertical axis belongs to.
I think something similar to this feature is useful to add the plotting of digital signals. It then allows you to organise the plot area, again similar to digital oscilloscopes.
Let me know what you think.