gr-qtgui: Add numeric entry widget

[eltos]

Description

The numeric entry widget combines the useful features of Range and Entry widgets and adds support for unit handling, numeric expressions in any common format and lower/upper bounds.

Examples (unit Hz):

"0.1" -> 0.1 Hz
"1e3" -> 1000 Hz
"5*2-3" -> 7 Hz
"8 kHz" -> 8000 Hz
"30m" -> 0.03 Hz

When the entry is focussed, the user can use the UP/DOWN keys to in-/decrement the value by the specified increment, and the PageUp/PageDown for ten times the increment.

The label color is used to indicate the status:
blue = editing but value not yet applied
red = invalid input
black = value applied

If the variable is changed from some other place in the flowgraph via the top block's "set_xxx()" callback, or if one of the boundaries (min/max value) changes, this is reflected on the UI

The precision defines the maximum decimal places to consider.

Related Issue

none

Which blocks/areas does this affect?

new block in gr-qtgui

Testing Done

In my own OOT. Added an example flow graph to this PR: gr-qtgui/examples/qtgui_numeric_entry.grc

Checklist

• I have read the CONTRIBUTING document.
• I have squashed my commits to have one significant change per commit.
• I have signed my commits before making this PR
• My code follows the code style of this project. See GREP1.md.
• I have updated the documentation where necessary.
• I have added tests to cover my changes, and all previous tests pass.

[willcode]

Please look into ast.literal_eval. We haven't kept eval() completely out of the code, but don't need to add any new ones. eval() allows for the execution of arbitary code, making it easier to create malicious grc files.

[eltos]

I'm aware of the security implications of eval, that's why there is a security check before evaluation. It only allows numbers (including the letter e for exponentials), brackets and the operators +-*/. Arbitrary code execution is thus not possible.

ast.literal_eval is only for literals. It does not allow evaluating expressions like 2**8 or 1e6-534, but I find it is a very useful feature to have a little "calculator" built in.

The "normal" Entry widget also uses eval without checks and allows for arbitrarily code execution. It enables doing maths with flow graph variables, and things like math.sin(2) which is handy at times. Nevertheless I think the security check before eval as in this PR is reasonable.

[haakov]

I haven't looked at the code, but I think this looks very neat UX-wise.

[willcode]

Other places in GRC that do eval() pass in a local namespace. If we do that, and specify __builtins__ = None, which we should probably be doing elsewhere, then it at least takes some weird code to cause trouble.

[eltos]

Other places in GRC that do eval() pass in a local namespace.

As mentioned before, QT GUI Entry is a counterexample: when type is set to raw, it calls eval(str(text)) without any safety here

If we do that, and specify builtins = None, which we should probably be doing elsewhere, then it at least takes some weird code to cause trouble.

@willcode Yes, but that helps nothing if you are really concerned about arbitrary code execution. Here's an example for "some weird code" which – if passed to eval(string, {"__builtins__":{}}) – can shut down your PC:

string = "[x().load_module('os').system('shutdown --help') for x in ().__class__.__base__.__subclasses__() if x.__name__=='BuiltinImporter']"

I added --help so it won't actually do that ;)

---

On the other hand, to bypass the filter I am using here, you would need a local or global name which consists of no other letter than e. This does not exist and would require that an attacker is already able to inject code.

if match := re.search(r'[^0-9.e+\-*/() ]', string):
    raise ValueError("Characters not allowed: " + match.group())
value = float(eval(string))

I consider this save

---

If you want it bullet-prove and avoid eval under all circumstances, we'd have to parse the string iteratively with AST and check each node, mimicking a simple calculator. But I think this is overkill

import ast
def save_numeric_eval(expression):
    save_operations = {
        ast.UAdd: lambda a: a,
        ast.USub: lambda a: -a,
        ast.Add: lambda a,b: a+b,
        ast.Sub: lambda a,b: a-b,
        ast.Mult: lambda a,b: a*b,
        ast.Div: lambda a,b: a/b,
        ast.Pow: lambda a,b: a**b,
    }
    def evaluate(node):
        if isinstance(node, ast.Expr):
            return evaluate(node.value)
        if isinstance(node, ast.Constant):
            return node.value
        if isinstance(node, ast.UnaryOp):
            if op := save_operations.get(node.op.__class__, None):
                return op(evaluate(node.operand))
        if isinstance(node, ast.BinOp):
            if op := save_operations.get(node.op.__class__, None):
                return op(evaluate(node.left), evaluate(node.right))
        raise TypeError("Unsupported operation: %s" % node.__class__.__name__)
    
    return float(evaluate(ast.parse(expression).body[-1]))

[willcode]

The eval should at least use a local namespace. I don't know to what degree that helps, but let's not be less "secure" than the rest of the system. Agree that there are ways around things, and there are other places in the system (e.g., embedded python blocks) that are worse.

The qtgui.tree.yml update is probably on your system but not checked in here, so the block does not show up in the GRC block list.

Need the DCO/signoff on commits.

Numbers are always floats, shown in exp format, which is generally not what the user wants to see. For example, if this is used for Hz, most people would expect to see m, k, M, G type notation or a raw number or at least an exp that's a multiple of 3. Something like that should at least be an option.

@haakov @dl1ksv if you could try this out and give some feedback, that would be great.

Should we just try to improve the existing entry block to support the features implemented here?

[eltos]

The eval should at least use a local namespace. I don't know to what degree that helps, but let's not be less "secure" than the rest of the system.

Not sure what else I can say to show that it helps nothing in terms of security; you have to rigorously filter the string (like it did) or use AST (which I did now to put your mind to ease).

The qtgui.tree.yml update is probably on your system but not checked in here, so the block does not show up in the GRC block list.
Need the DCO/signoff on commits.

Did signoff, apparently your bot does not like my GitHub usernames. Both should be fixed now.

Numbers are always floats, shown in exp format, which is generally not what the user wants to see. For example, if this is used for Hz, most people would expect to see m, k, M, G type notation or a raw number or at least an exp that's a multiple of 3. Something like that should at least be an option.

Agreed for labels, but not for entry fields. Imagine a field with unit "Hz" set to "30 Hz". The user types in "1000" which becomes "1 kHz". Then the user types in "1e6". Now, should this be interpreted in the base unit "Hz" and become "1 MHz" or in the previously shown unit "kHz" and become "1 GHz"? Either would be confusing from UX point of view. If the base unit is set to e.g. "MHz" then it could even be either of "Hz", "kHz" or "MHz". Sticking to a single unit avoids this ambiguity. And we don't want to force the user to type out the unit explicitly every time.
The number of digits after which "1234 Hz" is displayed as "1.2e3 Hz" can be controlled with the precision parameter.

[dl1ksv]

Just played a bit with the example.

That's what I got

Traceback (most recent call last):
File "/tmp/qtGuiNumericEntry.py", line 87, in
lambda: self.set_limit_max(eval(str(self._limit_max_line_edit.text()))))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "", line 0

SyntaxError: invalid syntax
Traceback (most recent call last):
File "/tmp/qtGuiNumericEntry.py", line 87, in
lambda: self.set_limit_max(eval(str(self._limit_max_line_edit.text()))))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "", line 0

SyntaxError: invalid syntax

Sorry, I don't know what I did.

[dl1ksv]

A small fm radio build on the funcube pro with QT Gui Range

and this with the numeric entry widget.

The numeric window looks nicer and fits better in the layout. (This may be a bug for the QTGui range ).

Both blocks are missing a msg input port, that one could set the value be clicking on the spectrum display.

But what is the real advantage of the numeric block against the range block ?
By the way I tested on F39

[willcode]

So, would it be possible to integrate the advantages of this block into the existing range or entry block, so we don't have multiple blocks that do almost the same thing?

One difference is that this block is Python-only.

[willcode]

Builds, install, works fine.

We do have a number of qtgui widgets that are Python-only, so that's only a minor point. Since the range block is also Python-only, I think we should add this functionality to the range block, rather than add another block.

Thank you for the AST calculator version of the code. Points well taken on the lack of security of other evals.

[dl1ksv]

Take the digital number control widget into consideration, too.

[eltos]

So, would it be possible to integrate the advantages of this block into the existing range or entry block, so we don't have multiple blocks that do almost the same thing? One difference is that this block is Python-only. We do have a number of qtgui widgets that are Python-only, so that's only a minor point. Since the range block is also Python-only, I think we should add this functionality to the range block, rather than add another block.

It would make sense, but Range also supports integers and has sliders, knobs and counters which one would have to handle properly (one important feature of this block is that ranges are runtime adjustable and optional). Also, this blocks widget reflects "external" changes of the variable done by other blocks, which Range currently does not.
It would require a lot of extra work to integrate it properly, which unfortunately I don't have time for at the moment.

[eltos]

Both blocks are missing a msg input port, that one could set the value be clicking on the spectrum display.

@dl1ksv at least for my block you can simply use a Message Pair to Var at the Frequency Sinks message output to update the variable. My widget recognizes such "external" changes of its variables value and will update the text accordingly. This also works for Entry widget btw (but not for Range widget).

[dl1ksv]

@eltos I see no message domain in your yaml definition. So your block has no message ports.

[willcode]

The idea above was that this would be done externally with msg-to-var. A msg port would be cleaner, though.

[eltos]

A msg port would be cleaner, though.

@willcode Can you provide a hint for msg ports on variable-defining blocks?
Since the block id and variable name are identical by design, GRC will generate a python file like this

# Variables
self.freq = freq = 5
# Blocks
self._freq_tool_bar = NumericEntry(....)
# Connections
self.msg_connect((...), (self.freq, 'msg'))

which of course will fail because self.freq is the variable, not the block.
How to tell GRC to use a custom block name to connect message ports? It looks like this is hard-coded in the generator.
I have seen some blocks like qtgui_msgcheckbox overwriting it like self.freq = self._freq_tool_bar to work around this, but that is everything but clean, as self.freq is expected to be the value (not the block) in other places. I've had some issues with this kind of widgets in the past and abandoned them.

It looks to me that a block which defines a variable and at the same time has ports can currently not be implemented in a clean way with GR, as its id is used for both, the variable and block name. Basically, as soon as a variable-defining block derives from gr.basic_block then not only ports, but also alias, affinity, min and maxoutbuf settings cause the same troubles.

I guess this makes for a dedicated issue, but in the present context using a separate Message Pair to Var block seams much cleaner, actually.