confserver: Add support for new V2 protocol

V2 adds:
* Independent result for visibility (showing/hiding menus)
* Includes adding IDs for all items (menus & symbols) in kconfig_menus.json

Still backwards compatible with V1, with some small changes (menu items now listed in results).

Also added some protocol docs, changed the "listening on stdin" message to come after any kconfiglib warnings

tools/kconfig_new/README.md

@@ -0,0 +1,99 @@
+# kconfig_new
+
+kconfig_new is the kconfig support used by the CMake-based build system.
+
+It uses a fork of [kconfiglib](https://github.com/ulfalizer/Kconfiglib) which adds a few small features (newer upstream kconfiglib also has the support we need, we just haven't updated yet). See comments at top of kconfiglib.py for details
+
+## confserver.py
+
+confserver.py is a small Python program intended to support IDEs and other clients who want to allow editing sdkconfig, without needing to reproduce all of the kconfig logic in a particular program.
+
+After launching confserver.py (which can be done via `idf.py confserver` command or `confserver` build target in ninja/make), the confserver communicates via JSON sent/received on stdout. Out-of-band errors are logged via stderr.
+
+### Configuration Structure
+
+During cmake run, the CMake-based build system produces a number of metadata files including `build/config/kconfig_menus.json`, which is a JSON representation of all the menu items in the project configuration and their structure.
+
+This format is currently undocumented, however running CMake with an IDF project will give an indication of the format. The format is expected to be stable.
+
+### Initial Process
+
+After initializing, the server will print "Server running, waiting for requests on stdin..." on stderr.
+
+Then it will print a JSON dictionary on stdout, representing the initial state of sdkconfig:
+
+```
+{
+  "version": 2,
+  "ranges": {
+               "TEST_CONDITIONAL_RANGES": [0, 10] },
+  "visible": { "TEST_CONDITIONAL_RANGES": true,
+                "CHOICE_A": true,
+                "test-config-submenu": true },
+  "values": { "TEST_CONDITIONAL_RANGES": 1,
+              "CHOICE_A": true },
+}
+```
+
+(Note: actual output is not pretty-printed and will print on a single line. Order of dictionary keys is undefined.)
+
+* "version" key is the protocol version in use.
+* "ranges" holds is a dictionary for any config symbol which has a valid integer range. The array value has two values for min/max.
+* "visible" holds a dictionary showing initial visibility status of config symbols (identified by the config symbol name) and menus (which don't represent a symbol but are represented as an id 'slug'). Both these names (symbol name and menu slug) correspond to the 'id' key in kconfig_menus.json.
+* "values" holds a dictionary showing initial values of all config symbols. Invisible symbols are not included here.
+
+### Interaction
+
+Interaction consists of the client sending JSON dictionary "requests" to the server one at a time. The server will respond to each request with a JSON dictionary response. Interaction is done when the client closes stdout (at this point the server will exit).
+
+Requests look like:
+
+```
+{ "version": 2,
+  "set": { "TEST_CHILD_STR": "New value",
+           "TEST_BOOL": true }
+}
+```
+
+Note: Requests don't need to be pretty-printed, they just need to be valid JSON.
+
+The `version` key *must* be present in the request and must match a protocol version supported by confserver.
+
+The `set` key is optional. If present, its value must be a dictionary of new values to set on kconfig symbols.
+
+Additional optional keys:
+
+* `load`: If this key is set, sdkconfig file will be reloaded from filesystem before any values are set applied. The value of this key can be a filename, in which case configuration will be loaded from this file. If the value of this key is `null`, configuration will be loaded from the last used file.
+
+* `save`: If this key is set, sdkconfig file will be saved after any values are set. Similar to `load`, the value of this key can be a filename to save to a particular file, or `null` to reuse the last used file.
+
+After a request is processed, a response is printed to stdout similar to this:
+
+```
+{ "version": 2,
+  "ranges": {},
+  "visible": { "test-config-submenu": false},
+  "values": { "SUBMENU_TRIGGER": false }
+}
+```
+
+* `version` is the protocol version used by the server.
+* `ranges` contains any changed ranges, where the new range of the config symbol has changed (due to some other configuration change or because a new sdkconfig has been loaded).
+* `visible` contains any visibility changes, where the visible config symbols have changed.
+* `values` contains any value changes, where a config symbol value has changed. This may be due to an explicit change (ie the client `set` this value), or a change caused by some other change in the config system. Note that a change which is set by the client may not be reflected exactly the same in the response, due to restrictions on allowed values which are enforced by the config server. Invalid changes are ignored by the config server.
+
+### Error Responses
+
+In some cases, a request may lead to an error message. In this case, the error message is printed to stderr but an array of errors is also returned in the `error` key of the response:
+
+```
+{ "version": 777,
+  "error": [ "Unsupported request version 777. Server supports versions 1-2" ]
+}
+```
+
+These error messages are intended to be human readable, not machine parseable.
+
+### Protocol Version Changes
+
+* V2: Added the `visible` key to the response. Invisible items are no longer represented as having value null.

tools/kconfig_new/confgen.py

@@ -27,6 +27,7 @@ import os
 import os.path
 import tempfile
 import json
+import re
 
 import gen_kconfig_doc
 import kconfiglib
@@ -184,7 +185,32 @@ def write_json(config, filename):
         json.dump(config_dict, f, indent=4, sort_keys=True)
 
 
+def get_menu_node_id(node):
+    """ Given a menu node, return a unique id
+    which can be used to identify it in the menu structure
+
+    Will either be the config symbol name, or a menu identifier
+    'slug'
+
+    """
+    try:
+        if not isinstance(node.item, kconfiglib.Choice):
+            return node.item.name
+    except AttributeError:
+        pass
+
+    result = []
+    while node.parent is not None:
+        slug = re.sub(r'\W+', '-', node.prompt[0]).lower()
+        result.append(slug)
+        node = node.parent
+
+    result = "-".join(reversed(result))
+    return result
+
+
 def write_json_menus(config, filename):
+    existing_ids = set()
     result = []  # root level items
     node_lookup = {}  # lookup from MenuNode to an item in result
 
@@ -211,7 +237,7 @@ def write_json_menus(config, filename):
             new_json = {"type": "menu",
                         "title": node.prompt[0],
                         "depends_on": depends,
-                        "children": []
+                        "children": [],
                         }
             if is_menuconfig:
                 sym = node.item
@@ -258,6 +284,12 @@ def write_json_menus(config, filename):
             }
 
         if new_json:
+            node_id = get_menu_node_id(node)
+            if node_id in existing_ids:
+                raise RuntimeError("Config file contains two items with the same id: %s (%s). " +
+                                   "Please rename one of these items to avoid ambiguity." % (node_id, node.prompt[0]))
+            new_json["id"] = node_id
+
             json_parent.append(new_json)
             node_lookup[node] = new_json
 

tools/kconfig_new/confserver.py

@@ -12,6 +12,10 @@ import sys
 import confgen
 from confgen import FatalError, __version__
 
+# Min/Max supported protocol versions
+MIN_PROTOCOL_VERSION = 1
+MAX_PROTOCOL_VERSION = 2
+
 
 def main():
     parser = argparse.ArgumentParser(description='confserver.py v%s - Config Generation Tool' % __version__, prog=os.path.basename(sys.argv[0]))
@@ -27,8 +31,19 @@ def main():
     parser.add_argument('--env', action='append', default=[],
                         help='Environment to set when evaluating the config file', metavar='NAME=VAL')
 
+    parser.add_argument('--version', help='Set protocol version to use on initial status',
+                        type=int, default=MAX_PROTOCOL_VERSION)
+
     args = parser.parse_args()
 
+    if args.version < MIN_PROTOCOL_VERSION:
+        print("Version %d is older than minimum supported protocol version %d. Client is much older than ESP-IDF version?" %
+              (args.version, MIN_PROTOCOL_VERSION))
+
+    if args.version > MAX_PROTOCOL_VERSION:
+        print("Version %d is newer than maximum supported protocol version %d. Client is newer than ESP-IDF version?" %
+              (args.version, MAX_PROTOCOL_VERSION))
+
     try:
         args.env = [(name,value) for (name,value) in (e.split("=",1) for e in args.env)]
     except ValueError:
@@ -38,17 +53,26 @@ def main():
     for name, value in args.env:
         os.environ[name] = value
 
-    print("Server running, waiting for requests on stdin...", file=sys.stderr)
     run_server(args.kconfig, args.config)
 
 
-def run_server(kconfig, sdkconfig):
+def run_server(kconfig, sdkconfig, default_version=MAX_PROTOCOL_VERSION):
     config = kconfiglib.Kconfig(kconfig)
     config.load_config(sdkconfig)
 
+    print("Server running, waiting for requests on stdin...", file=sys.stderr)
+
     config_dict = confgen.get_json_values(config)
     ranges_dict = get_ranges(config)
-    json.dump({"version": 1, "values": config_dict, "ranges": ranges_dict}, sys.stdout)
+    visible_dict = get_visible(config)
+
+    if default_version == 1:
+        # V1: no 'visibility' key, send value None for any invisible item
+        values_dict = dict((k, v if visible_dict[k] else False) for (k,v) in config_dict.items())
+        json.dump({"version": 1, "values": values_dict, "ranges": ranges_dict}, sys.stdout)
+    else:
+        # V2 onwards: separate visibility from version
+        json.dump({"version": default_version, "values": config_dict, "ranges": ranges_dict, "visible": visible_dict}, sys.stdout)
     print("\n")
 
     while True:
@@ -58,10 +82,12 @@ def run_server(kconfig, sdkconfig):
         req = json.loads(line)
         before = confgen.get_json_values(config)
         before_ranges = get_ranges(config)
+        before_visible = get_visible(config)
 
         if "load" in req:  # if we're loading a different sdkconfig, response should have all items in it
             before = {}
             before_ranges = {}
+            before_visible = {}
 
             # if no new filename is supplied, use existing sdkconfig path, otherwise update the path
             if req["load"] is None:
@@ -79,10 +105,19 @@ def run_server(kconfig, sdkconfig):
 
         after = confgen.get_json_values(config)
         after_ranges = get_ranges(config)
+        after_visible = get_visible(config)
 
         values_diff = diff(before, after)
         ranges_diff = diff(before_ranges, after_ranges)
-        response = {"version": 1, "values": values_diff, "ranges": ranges_diff}
+        visible_diff = diff(before_visible, after_visible)
+        if req["version"] == 1:
+            # V1 response, invisible items have value None
+            for k in (k for (k,v) in visible_diff.items() if not v):
+                values_diff[k] = None
+            response = {"version": 1, "values": values_diff, "ranges": ranges_diff}
+        else:
+            # V2+ response, separate visibility values
+            response = {"version": req["version"], "values": values_diff, "ranges": ranges_diff, "visible": visible_diff}
         if error:
             for e in error:
                 print("Error: %s" % e, file=sys.stderr)
@@ -94,8 +129,12 @@ def run_server(kconfig, sdkconfig):
 def handle_request(config, req):
     if "version" not in req:
         return ["All requests must have a 'version'"]
-    if int(req["version"]) != 1:
-        return ["Only version 1 requests supported"]
+
+    if req["version"] < MIN_PROTOCOL_VERSION or req["version"] > MAX_PROTOCOL_VERSION:
+        return ["Unsupported request version %d. Server supports versions %d-%d" % (
+            req["version"],
+            MIN_PROTOCOL_VERSION,
+            MAX_PROTOCOL_VERSION)]
 
     error = []
 
@@ -154,12 +193,10 @@ def handle_set(config, error, to_set):
 
 def diff(before, after):
     """
-    Return a dictionary with the difference between 'before' and 'after' (either with the new value if changed,
-    or None as the value if a key in 'before' is missing in 'after'
+    Return a dictionary with the difference between 'before' and 'after',
+    for items which are present in 'after' dictionary
     """
     diff = dict((k,v) for (k,v) in after.items() if before.get(k, None) != v)
-    hidden = dict((k,None) for k in before if k not in after)
-    diff.update(hidden)
     return diff
 
 
@@ -178,6 +215,35 @@ def get_ranges(config):
     return ranges_dict
 
 
+def get_visible(config):
+    """
+    Return a dict mapping node IDs (config names or menu node IDs) to True/False for their visibility
+    """
+    result = {}
+    menus = []
+
+    # when walking the menu the first time, only
+    # record whether the config symbols are visible
+    # and make a list of menu nodes (that are not symbols)
+    def handle_node(node):
+        sym = node.item
+        try:
+            visible = (sym.visibility != 0)
+            result[node] = visible
+        except AttributeError:
+            menus.append(node)
+    config.walk_menu(handle_node)
+
+    # now, figure out visibility for each menu. A menu is visible if any of its children are visible
+    for m in reversed(menus):  # reverse to start at leaf nodes
+        result[m] = any(v for (n,v) in result.items() if n.parent == m)
+
+    # return a dict mapping the node ID to its visibility.
+    result = dict((confgen.get_menu_node_id(n),v) for (n,v) in result.items())
+
+    return result
+
+
 if __name__ == '__main__':
     try:
         main()

tools/kconfig_new/test/Kconfig

@@ -41,4 +41,32 @@ menu "Test config"
         range 0 10
         default 1
 
-endmenu
+    config SUBMENU_TRIGGER
+        bool "I enable/disable some submenu items"
+        default y
+
+    menu "Submenu"
+
+        config SUBMENU_ITEM_A
+            int "I am a submenu item"
+            depends on SUBMENU_TRIGGER
+            default 77
+
+        config SUBMENU_ITEM_B
+            bool "I am also submenu item"
+            depends on SUBMENU_TRIGGER
+
+    endmenu  # Submenu
+
+    menuconfig SUBMENU_CONFIG
+        bool "Submenuconfig"
+        default y
+        help
+            I am a submenu which is also a config item.
+
+    config SUBMENU_CONFIG_ITEM
+        bool "Depends on submenuconfig"
+        depends on SUBMENU_CONFIG
+        default y
+
+endmenu  # Test config

tools/kconfig_new/test/README.md

@@ -0,0 +1,32 @@
+# KConfig Tests
+
+## confserver.py tests
+
+Install pexpect (`pip install pexpect`).
+
+Then run the tests manually like this:
+
+```
+./test_confserver.py --logfile tests.log
+```
+
+If a weird error message comes up from the test, check the log file (`tests.log`) which has the full interaction session (input and output) from confserver.py - sometimes the test suite misinterprets some JSON-like content in a Python error message as JSON content.
+
+Note: confserver.py prints its error messages on stderr, to avoid overlap with JSON content on stdout. However pexpect uses a pty (virtual terminal) which can't distinguish stderr and stdout.
+
+Test cases apply to `KConfig` config schema. Cases are listed in `testcases.txt` and are each of this form:
+
+```
+* Set TEST_BOOL, showing child items
+> { "TEST_BOOL" : true }
+< { "values" : { "TEST_BOOL" : true, "TEST_CHILD_STR" : "OHAI!", "TEST_CHILD_BOOL" : true }, "ranges": {"TEST_CONDITIONAL_RANGES": [0, 100]}, "visible": {"TEST_CHILD_BOOL" : true, "TEST_CHILD_STR" : true} }
+
+```
+
+* First line (`*`) is description
+* Second line (`>`) is changes to send
+* Third line (`<`) is response to expect back
+* (Blank line between cases)
+
+Test cases are run in sequence, so any test case depends on the state changes caused by all items above it.
+

tools/kconfig_new/test/test_confserver.py

@@ -7,9 +7,12 @@ import tempfile
 
 import pexpect
 
+# Each protocol version to be tested needs a 'testcases_vX.txt' file
+PROTOCOL_VERSIONS = [1, 2]
 
-def parse_testcases():
-    with open("testcases.txt", "r") as f:
+
+def parse_testcases(version):
+    with open("testcases_v%d.txt" % version, "r") as f:
         cases = [l for l in f.readlines() if len(l.strip()) > 0]
     # Each 3 lines in the file should be formatted as:
     # * Description of the test change
@@ -52,46 +55,17 @@ def main():
         p.logfile = args.logfile
         p.setecho(False)
 
-        def expect_json():
-            # run p.expect() to expect a json object back, and return it as parsed JSON
-            p.expect("{.+}\r\n")
-            return json.loads(p.match.group(0).strip().decode())
-
         p.expect("Server running.+\r\n")
-        initial = expect_json()
+        initial = expect_json(p)
         print("Initial: %s" % initial)
-        cases = parse_testcases()
-
-        for (desc, send, expected) in cases:
-            print(desc)
-            req = {"version": "1", "set": send}
-            req = json.dumps(req)
-            print("Sending: %s" % (req))
-            p.send("%s\n" % req)
-            readback = expect_json()
-            print("Read back: %s" % (json.dumps(readback)))
-            if readback.get("version", None) != 1:
-                raise RuntimeError('Expected {"version" : 1} in response')
-            for expect_key in expected.keys():
-                read_vals = readback[expect_key]
-                exp_vals = expected[expect_key]
-                if read_vals != exp_vals:
-                    expect_diff = dict((k,v) for (k,v) in exp_vals.items() if k not in read_vals or v != read_vals[k])
-                    raise RuntimeError("Test failed! Was expecting %s: %s" % (expect_key, json.dumps(expect_diff)))
-            print("OK")
-
-        print("Testing load/save...")
-        before = os.stat(temp_sdkconfig_path).st_mtime
-        p.send("%s\n" % json.dumps({"version": "1", "save": temp_sdkconfig_path}))
-        save_result = expect_json()
-        print("Save result: %s" % (json.dumps(save_result)))
-        assert len(save_result["values"]) == 0
-        assert len(save_result["ranges"]) == 0
-        after = os.stat(temp_sdkconfig_path).st_mtime
-        assert after > before
-
-        p.send("%s\n" % json.dumps({"version": "1", "load": temp_sdkconfig_path}))
-        load_result = expect_json()
+
+        for version in PROTOCOL_VERSIONS:
+            test_protocol_version(p, version)
+
+        test_load_save(p, temp_sdkconfig_path)
+
+        p.send("%s\n" % json.dumps({"version": 2, "load": temp_sdkconfig_path}))
+        load_result = expect_json(p)
         print("Load result: %s" % (json.dumps(load_result)))
         assert len(load_result["values"]) > 0  # loading same file should return all config items
         assert len(load_result["ranges"]) > 0
@@ -104,5 +78,68 @@ def main():
             pass
 
 
+def expect_json(p):
+    # run p.expect() to expect a json object back, and return it as parsed JSON
+    p.expect("{.+}\r\n")
+    result = p.match.group(0).strip().decode()
+    print("Read raw data from server: %s" % result)
+    return json.loads(result)
+
+
+def send_request(p, req):
+    req = json.dumps(req)
+    print("Sending: %s" % (req))
+    p.send("%s\n" % req)
+    readback = expect_json(p)
+    print("Read back: %s" % (json.dumps(readback)))
+    return readback
+
+
+def test_protocol_version(p, version):
+    print("*****")
+    print("Testing version %d..." % version)
+
+    # reload the config from the sdkconfig file
+    req = {"version": version, "load": None}
+    readback = send_request(p, req)
+    print("Reset response: %s" % (json.dumps(readback)))
+
+    # run through each test case
+    cases = parse_testcases(version)
+    for (desc, send, expected) in cases:
+        print(desc)
+        req = {"version": version, "set": send}
+        readback = send_request(p, req)
+        if readback.get("version", None) != version:
+            raise RuntimeError('Expected {"version" : %d} in response' % version)
+        for expect_key in expected.keys():
+            read_vals = readback[expect_key]
+            exp_vals = expected[expect_key]
+            if read_vals != exp_vals:
+                expect_diff = dict((k,v) for (k,v) in exp_vals.items() if k not in read_vals or v != read_vals[k])
+                raise RuntimeError("Test failed! Was expecting %s: %s" % (expect_key, json.dumps(expect_diff)))
+        print("OK")
+    print("Version %d OK" % version)
+
+
+def test_load_save(p, temp_sdkconfig_path):
+    print("Testing load/save...")
+    before = os.stat(temp_sdkconfig_path).st_mtime
+    save_result = send_request(p, {"version": 2, "save": temp_sdkconfig_path})
+    print("Save result: %s" % (json.dumps(save_result)))
+    assert "error" not in save_result
+    assert len(save_result["values"]) == 0  # nothing changes when we save
+    assert len(save_result["ranges"]) == 0
+    after = os.stat(temp_sdkconfig_path).st_mtime
+    assert after > before  # something got written to disk
+
+    # Do a V1 load
+    load_result = send_request(p, {"version": 1, "load": temp_sdkconfig_path})
+    print("V1 Load result: %s" % (json.dumps(load_result)))
+    assert "error" not in load_result
+    assert len(load_result["values"]) > 0  # in V1, loading same file should return all config items
+    assert len(load_result["ranges"]) > 0
+
+
 if __name__ == "__main__":
     main()

tools/kconfig_new/test/testcases_v2.txt

@@ -0,0 +1,47 @@
+* Set TEST_BOOL, showing child items
+> { "TEST_BOOL" : true }
+< { "values" : { "TEST_BOOL" : true, "TEST_CHILD_STR" : "OHAI!", "TEST_CHILD_BOOL" : true }, "ranges": {"TEST_CONDITIONAL_RANGES": [0, 100]}, "visible": {"TEST_CHILD_BOOL" : true, "TEST_CHILD_STR" : true} }
+
+* Set TEST_CHILD_STR
+> { "TEST_CHILD_STR" : "Other value" }
+< { "values" : { "TEST_CHILD_STR" : "Other value" } }
+
+* Clear TEST_BOOL, hiding child items
+> { "TEST_BOOL" : false }
+< { "values" : { "TEST_BOOL" : false },  "ranges": {"TEST_CONDITIONAL_RANGES": [0, 10]}, "visible": { "TEST_CHILD_BOOL" : false, "TEST_CHILD_STR" : false } }
+
+* Set TEST_CHILD_BOOL, invalid as parent is disabled
+> { "TEST_CHILD_BOOL" : false }
+< { "values" : { } }
+
+* Set TEST_BOOL & TEST_CHILD_STR together
+> { "TEST_BOOL" : true, "TEST_CHILD_STR" : "New value" }
+< { "values" : { "TEST_BOOL" : true, "TEST_CHILD_STR" : "New value", "TEST_CHILD_BOOL" : true } }
+
+* Set choice
+> { "CHOICE_B" : true }
+< { "values" : { "CHOICE_B" : true, "CHOICE_A" : false, "DEPENDS_ON_CHOICE" : "Depends on B" } }
+
+* Set string which depends on choice B
+> { "DEPENDS_ON_CHOICE" : "oh, really?" }
+< { "values" : { "DEPENDS_ON_CHOICE" : "oh, really?" } }
+
+* Try setting boolean values to invalid types
+> { "CHOICE_A" : 11, "TEST_BOOL" : "false" }
+< { "values" : { } }
+
+* Disabling all items in a submenu causes all sub-items to have visible:False
+> { "SUBMENU_TRIGGER": false }
+< { "values" : { "SUBMENU_TRIGGER": false}, "visible": { "test-config-submenu" : false, "SUBMENU_ITEM_A": false, "SUBMENU_ITEM_B": false} }
+
+* Re-enabling submenu causes that menu to be visible again, and refreshes sub-items
+> { "SUBMENU_TRIGGER": true }
+< { "values" : { "SUBMENU_TRIGGER": true}, "visible": {"test-config-submenu": true, "SUBMENU_ITEM_A": true, "SUBMENU_ITEM_B": true}, "values": {"SUBMENU_TRIGGER": true, "SUBMENU_ITEM_A": 77, "SUBMENU_ITEM_B": false } }
+
+* Disabling submenuconfig item hides its children
+> { "SUBMENU_CONFIG": false }
+< { "values" : { "SUBMENU_CONFIG": false }, "visible": { "SUBMENU_CONFIG_ITEM": false } }
+
+* Enabling submenuconfig item re-shows its children
+> { "SUBMENU_CONFIG": true }
+< { "values" : { "SUBMENU_CONFIG_ITEM": true, "SUBMENU_CONFIG" : true }, "visible": { "SUBMENU_CONFIG_ITEM": true } }