From 90ddbaac16993e5f10c3891f3b2b4d76a29d0aab Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 25 Jan 2019 12:16:51 +0800 Subject: [PATCH 1/3] 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 | 99 ++++++++++++++++ tools/kconfig_new/confgen.py | 34 +++++- tools/kconfig_new/confserver.py | 86 ++++++++++++-- tools/kconfig_new/test/Kconfig | 30 ++++- tools/kconfig_new/test/README.md | 32 +++++ tools/kconfig_new/test/test_confserver.py | 111 ++++++++++++------ .../test/{testcases.txt => testcases_v1.txt} | 0 tools/kconfig_new/test/testcases_v2.txt | 47 ++++++++ 8 files changed, 390 insertions(+), 49 deletions(-) create mode 100644 tools/kconfig_new/README.md create mode 100644 tools/kconfig_new/test/README.md rename tools/kconfig_new/test/{testcases.txt => testcases_v1.txt} (100%) create mode 100644 tools/kconfig_new/test/testcases_v2.txt diff --git a/tools/kconfig_new/README.md b/tools/kconfig_new/README.md new file mode 100644 index 000000000..2410b166b --- /dev/null +++ b/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. diff --git a/tools/kconfig_new/confgen.py b/tools/kconfig_new/confgen.py index 499e64eaa..8bb0d33da 100755 --- a/tools/kconfig_new/confgen.py +++ b/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 diff --git a/tools/kconfig_new/confserver.py b/tools/kconfig_new/confserver.py index 4de2757eb..5b94b90b3 100755 --- a/tools/kconfig_new/confserver.py +++ b/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() diff --git a/tools/kconfig_new/test/Kconfig b/tools/kconfig_new/test/Kconfig index 69500889b..6462fe82d 100644 --- a/tools/kconfig_new/test/Kconfig +++ b/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 diff --git a/tools/kconfig_new/test/README.md b/tools/kconfig_new/test/README.md new file mode 100644 index 000000000..ca7416e30 --- /dev/null +++ b/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. + diff --git a/tools/kconfig_new/test/test_confserver.py b/tools/kconfig_new/test/test_confserver.py index d772b1218..6adb6ffa5 100755 --- a/tools/kconfig_new/test/test_confserver.py +++ b/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") + for version in PROTOCOL_VERSIONS: + test_protocol_version(p, version) - 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 + test_load_save(p, temp_sdkconfig_path) - p.send("%s\n" % json.dumps({"version": "1", "load": temp_sdkconfig_path})) - load_result = expect_json() + 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() diff --git a/tools/kconfig_new/test/testcases.txt b/tools/kconfig_new/test/testcases_v1.txt similarity index 100% rename from tools/kconfig_new/test/testcases.txt rename to tools/kconfig_new/test/testcases_v1.txt diff --git a/tools/kconfig_new/test/testcases_v2.txt b/tools/kconfig_new/test/testcases_v2.txt new file mode 100644 index 000000000..77d23bf66 --- /dev/null +++ b/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 } } From ec4c75b692d7f04d45596b84c6849eb1ea6eb36a Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Thu, 14 Mar 2019 17:47:08 +1100 Subject: [PATCH 2/3] confserver: In protocol V2, a "load" should only send back changes not all items --- tools/kconfig_new/README.md | 1 + tools/kconfig_new/confserver.py | 12 ++++++++---- tools/kconfig_new/test/test_confserver.py | 12 +++++++----- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/tools/kconfig_new/README.md b/tools/kconfig_new/README.md index 2410b166b..b03ffd327 100644 --- a/tools/kconfig_new/README.md +++ b/tools/kconfig_new/README.md @@ -97,3 +97,4 @@ 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. +* V2: `load` now sends changes compared to values before the load, not the whole list of config items. diff --git a/tools/kconfig_new/confserver.py b/tools/kconfig_new/confserver.py index 5b94b90b3..3373a9d0c 100755 --- a/tools/kconfig_new/confserver.py +++ b/tools/kconfig_new/confserver.py @@ -84,10 +84,14 @@ def run_server(kconfig, sdkconfig, default_version=MAX_PROTOCOL_VERSION): 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 "load" in req: # load a new sdkconfig + + if req.get("version", default_version) == 1: + # for V1 protocol, send all items when loading new sdkconfig. + # (V2+ will only send changes, same as when setting an item) + before = {} + before_ranges = {} + before_visible = {} # if no new filename is supplied, use existing sdkconfig path, otherwise update the path if req["load"] is None: diff --git a/tools/kconfig_new/test/test_confserver.py b/tools/kconfig_new/test/test_confserver.py index 6adb6ffa5..53cad5b7c 100755 --- a/tools/kconfig_new/test/test_confserver.py +++ b/tools/kconfig_new/test/test_confserver.py @@ -64,11 +64,6 @@ def main(): 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 print("Done. All passed.") finally: @@ -133,6 +128,13 @@ def test_load_save(p, temp_sdkconfig_path): after = os.stat(temp_sdkconfig_path).st_mtime assert after > before # something got written to disk + # Do a V2 load + load_result = send_request(p, {"version": 2, "load": temp_sdkconfig_path}) + print("V2 Load result: %s" % (json.dumps(load_result))) + assert "error" not in load_result + assert len(load_result["values"]) == 0 # in V2, loading same file should return no config items + assert len(load_result["ranges"]) == 0 + # Do a V1 load load_result = send_request(p, {"version": 1, "load": temp_sdkconfig_path}) print("V1 Load result: %s" % (json.dumps(load_result))) From 2f2f0fbcbd5acb60af36220b413731a5a8e91c1d Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 15 Mar 2019 12:17:14 +1100 Subject: [PATCH 3/3] confserver: Send an error response if JSON request is malformatted --- tools/kconfig_new/confserver.py | 8 +++++++- tools/kconfig_new/test/test_confserver.py | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tools/kconfig_new/confserver.py b/tools/kconfig_new/confserver.py index 3373a9d0c..fcb33e63d 100755 --- a/tools/kconfig_new/confserver.py +++ b/tools/kconfig_new/confserver.py @@ -79,7 +79,13 @@ def run_server(kconfig, sdkconfig, default_version=MAX_PROTOCOL_VERSION): line = sys.stdin.readline() if not line: break - req = json.loads(line) + try: + req = json.loads(line) + except ValueError as e: # json module throws JSONDecodeError (sublcass of ValueError) on Py3 but ValueError on Py2 + response = {"version": default_version, "error": ["JSON formatting error: %s" % e]} + json.dump(response, sys.stdout) + print("\n") + continue before = confgen.get_json_values(config) before_ranges = get_ranges(config) before_visible = get_visible(config) diff --git a/tools/kconfig_new/test/test_confserver.py b/tools/kconfig_new/test/test_confserver.py index 53cad5b7c..d5da31913 100755 --- a/tools/kconfig_new/test/test_confserver.py +++ b/tools/kconfig_new/test/test_confserver.py @@ -64,6 +64,8 @@ def main(): test_load_save(p, temp_sdkconfig_path) + test_invalid_json(p) + print("Done. All passed.") finally: @@ -143,5 +145,21 @@ def test_load_save(p, temp_sdkconfig_path): assert len(load_result["ranges"]) > 0 +def test_invalid_json(p): + print("Testing invalid JSON formatting...") + + bad_escaping = r'{ "version" : 2, "load" : "c:\some\path\not\escaped\as\json" }' + p.send("%s\n" % bad_escaping) + readback = expect_json(p) + print(readback) + assert "json" in readback["error"][0].lower() + + not_really_json = 'Hello world!!' + p.send("%s\n" % not_really_json) + readback = expect_json(p) + print(readback) + assert "json" in readback["error"][0].lower() + + if __name__ == "__main__": main()