Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

emanesh bool conversion of runtime modifiable configuration not working #14

Closed
sgalgano opened this issue Aug 27, 2014 · 2 comments
Closed
Assignees

Comments

@sgalgano
Copy link
Member

Reported via email by Leonid Veytser:

There appears to be a bug in setting a boolean runtime modifiable configuration to value of False. Here's a small example of trying to set "enablepromiscuousmode" to False in RFPipe via emanesh:

[emanesh (localhost:47000)] ## get config 1 mac enablepromiscuousmode
nem 1 mac enablepromiscuousmode = False
[emanesh (localhost:47000)] ## set config 1 mac enablepromiscuousmode=True
nem 1 mac configuration updated
[emanesh (localhost:47000)] ## get config 1 mac enablepromiscuousmode
nem 1 mac enablepromiscuousmode = True
[emanesh (localhost:47000)] ## set config 1 mac enablepromiscuousmode=False
nem 1 mac configuration updated
[emanesh (localhost:47000)] ## get config 1 mac enablepromiscuousmode
nem 1 mac enablepromiscuousmode = True

As you can see, setting the value to True appears to be working but setting it back to False doesn't.

@sgalgano sgalgano self-assigned this Aug 27, 2014
@sgalgano
Copy link
Member Author

There is a bug in how emanesh converts configuration values to bool. Currently it uses bool() which is a bad idea since all non empty string values are converted to True and only empty strings are converted to False:

In [1]: bool('True')
Out[1]: True

In [2]: bool('False')
Out[2]: True

In [3]: bool('')
Out[3]: False

Here is a proposed fix:

diff --git a/src/emanesh/emanesh/emaneshell.py b/src/emanesh/emanesh/emaneshell.py
index 555f510..79f3bd9 100644
--- a/src/emanesh/emanesh/emaneshell.py
+++ b/src/emanesh/emanesh/emaneshell.py
@@ -752,6 +752,17 @@ class EMANEShell(cmd.Cmd):
             if len(args) > index:
                 for expression in args[index:]:
                     m = re.match('^([0-9A-Za-z]+)=(.+)', expression)
+
+                    def toBool(val):
+                        val = val.lower()
+
+                        if val in ('yes','on','enable','true','1'):
+                            return True
+                        elif  val in ('no','off','disable','false','0'):
+                            return False
+                        else:
+                            raise ValueError()
+                        
                     convert = {'uint64' : (ControlPortClient.TYPE_UINT64,long),
                                'uint32' : (ControlPortClient.TYPE_UINT32,long),
                                'uint16' : (ControlPortClient.TYPE_UINT16,long),
@@ -760,7 +771,7 @@ class EMANEShell(cmd.Cmd):
                                'int32' : (ControlPortClient.TYPE_INT32,long),
                                'int16' : (ControlPortClient.TYPE_INT16,long),
                                'int8' : (ControlPortClient.TYPE_INT8,long),
-                               'bool' : (ControlPortClient.TYPE_BOOLEAN,bool),
+                               'bool' : (ControlPortClient.TYPE_BOOLEAN,toBool),
                                'string': (ControlPortClient.TYPE_STRING,str),
                                'inetaddr' : (ControlPortClient.TYPE_INETADDR,str),
                                'float' : (ControlPortClient.TYPE_FLOAT,float),

sgalgano added a commit that referenced this issue Aug 27, 2014
replacing bool() usage with a function that checks for specific string
values in order to determine True or False.

bool() will evaluate to True for all non empty strings.

Reported-by: Leonid Veytser <veytser@ll.mit.edu>
See #14
@sgalgano
Copy link
Member Author

Fixed in eb4c5e5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant