sigrokproject / pulseview

Read-only mirror of the official repo at git://sigrok.org/pulseview. Pull requests welcome. Please file bugreports at sigrok.org/bugzilla.
http://sigrok.org/wiki/PulseView
GNU General Public License v3.0
487 stars 157 forks source link

Created new logic math signal #42

Open KyleJ61782 opened 3 years ago

KyleJ61782 commented 3 years ago

Added a logic math signal so that logic signals can be combined using an expression. This can be particularly useful when wanting to visually identify interesting locations to investigate within a series of logic traces.

This was accomplished by refactoring the math signal into mostly generic code with descendants that flesh out the analog and logic specific code.

abraxa commented 3 years ago

Hello. I appreciate that you spent time on this but unfortunately, this is not merge-able. I really wish you had contacted me first to discuss what you're going to do.

For example, the underlying math library exprtk supports boolean operators, so it makes sense to add support for logic signals in the math signal. Sure, the math signal generates an analog signal but that can easily be converted back to a logic signal using a conversion. If that's too inconvenient, an automatic conversion than be added. With that, I don't see the need for separate classes.

Also, you're violating the coding style by putting code in header files and you're placing the signal colors in files where they don't belong. data/analog.cpp and data/logic.cpp are merely holding data and are separate from signals.

Again, I wish you had spoken to me beforehand so that we could've aligned instead of ending up in this unfortunate situation.

KyleJ61782 commented 3 years ago

No worries! I provided the code in case it's useful. I'd certainly be open to reimplementing according to any and all suggestions you have. I agree that it's not the cleanest solution, for sure.

It's been a while, but I seem to recall one of the difficulties causing me to implement two separate classes was the fact that analog and logic signals have completely different storage underpinnings. However, any and all suggestions for proper implementation are very much so of interest.

depili commented 2 years ago

Sometimes it would be useful to be able to for example invert a logic signal before passing it to another existing decoder. I don't know if math signals would allow such a thing generally, as I only have digital signals to work with. If the current implementation can do such things it would be nice to have an example.

Another use-case would be dealing with differential digital signals and trying to filter out noise.