Merge pull request #162 from stanford-oval/wip/device-selector

Extend device selectors to refer to devices by name

lib/ast/api.js

@@ -34,9 +34,10 @@ const {
     recursiveYieldArraySlots,
     makeScope,
     InputParamSlot,
+    DeviceAttributeSlot,
     FilterSlot,
     ArrayIndexSlot,
-    FieldSlot
+    FieldSlot,
 } = require('./slots');
 
 const InvocationProto = Object.getPrototypeOf(new Ast.Invocation(Ast.Selector.Builtin, 'notify', [], null));
@@ -464,8 +465,16 @@ function* iterateSlots2InputParams(prim, scope) {
  * @yields {Ast~AbstractSlot}
  */
 InvocationProto.iterateSlots2 = function* iterateSlots2(scope) {
-    if (this.selector.isDevice)
+    if (this.selector.isDevice) {
+        for (let attr of this.selector.attributes)
+            yield new DeviceAttributeSlot(this, attr);
+
+        // note that we yield the selector after the device attributes
+        // this way, almond-dialog-agent will first ask any question to slot-fill
+        // the device attributes (if somehow it needs to) and then use the chosen
+        // device attributes to choose the device
         yield this.selector;
+    }
     return yield* iterateSlots2InputParams(this, scope);
 };
 
@@ -1052,7 +1061,7 @@ function lowerReturnAction(state, action, lastPrimitive, principal) {
     ];
     for (let name in lastPrimitive.schema.out)
         sendInputs.push(Ast.InputParam(name, Ast.Value.VarRef(name)));
-    action.invocation.selector = Ast.Selector.Device(localClass.name, null, null);
+    action.invocation.selector = new Ast.Selector.Device(localClass.name, null, null);
     action.invocation.channel = 'send';
     action.invocation.in_params = sendInputs;
     action.invocation.schema = sendSchema;
@@ -1065,7 +1074,7 @@ function lowerReturnAction(state, action, lastPrimitive, principal) {
     ];
     let receiveTrigger = new Ast.Stream.Monitor(
         new Ast.Table.Invocation(
-            new Ast.Invocation(Ast.Selector.Device(toSendClass.name, null, null), 'receive', receiveInputs, receiveSchema),
+            new Ast.Invocation(new Ast.Selector.Device(toSendClass.name, null, null), 'receive', receiveInputs, receiveSchema),
         receiveSchema),
     null, receiveSchema);
 

lib/ast/class_def.js

@@ -14,7 +14,7 @@ const assert = require('assert');
 //const Ast = require('./ast');
 const Type = require('../type');
 const { prettyprintClassDef } = require('../prettyprint');
-const { clean } = require('../utils');
+const { clean, cleanKind } = require('../utils');
 const { Value } = require('./values');
 const { Statement, InputParam } = require('./program');
 const { FunctionDef } = require('./function_def');
@@ -60,6 +60,10 @@ class ClassDef extends Statement {
         this.is_abstract = is_abstract || false;
     }
 
+    get canonical() {
+        return this.metadata.canonical || cleanKind(this.kind);
+    }
+
     *iterateSlots() {}
     *iteratePrimitives() {}
 

lib/ast/program.js

@@ -27,48 +27,85 @@ const {
  * Selectors correspond to the `@`-device part of the ThingTalk code,
  * up to but not including the function name.
  *
- * @class
  * @alias Ast.Selector
+ * @property {boolean} isSelector - true
  * @property {boolean} isDevice - true if this is an instance of {@link Ast.Selector.Device}
  * @property {boolean} isBuiltin - true if this is {@link Ast.Selector.Builtin}
  * @abstract
  */
-const Selector = adt.data(/** @lends Ast.Selector */ {
+class Selector {}
+Selector.prototype.isSelector = true;
+
+/**
+ * A selector that maps to one or more devices in Thingpedia.
+ *
+ * @alias Ast.Selector.Device
+ * @extends Ast.Selector
+ */
+class DeviceSelector extends Selector {
     /**
-     * A selector that maps to one or more devices in Thingpedia.
+     * Construct a new device selector.
      *
-     * @class
-     * @extends Ast.Selector
      * @param {string} kind - the Thingpedia class ID
      * @param {string|null} id - the unique ID of the device being selected, or null
-     *                           to select all devices
+     *                           to select devices according to the attributes, or
+     *                           all devices if no attributes are specified
      * @param {null} principal - reserved/deprecated, must be `null`
+     * @param {Ast.InputParam[]} attributes - other attributes used to select a device, if ID is unspecified
+     * @param {boolean} [all=false] - operate on all devices that match the attributes, instead of
+     *                                having the user choose
      */
-    Device: /** @lends Ast.Selector.Device.prototype */ {
-        /**
-         * The Thingpedia class ID to choose.
-         * @type {string}
-         * @readonly
-         */
-        kind: adt.only(String),
-        /**
-         * The unique ID of the device being selected.
-         * @type {string|null}
-         * @readonly
-         */
-        id: adt.only(String, null),
-        principal: adt.only(null),
-    },
-    /**
-     * A selector that maps the builtin `notify`, `return` and `save` functions.
-     *
-     * This is a singleton, not a class.
-     * @type {Ast.Selector}
-     * @readonly
-     */
-    Builtin: null
-});
-module.exports.Selector = Selector.seal();
+    constructor(kind, id, principal, attributes = [], all = false) {
+        super();
+
+        assert(typeof kind === 'string');
+        this.kind = kind;
+
+        assert(typeof id === 'string' || id === null);
+        this.id = id;
+
+        assert(principal === null);
+        this.principal = principal;
+
+        this.attributes = attributes;
+
+        this.all = all;
+    }
+
+    clone() {
+        const attributes = this.attributes.map((attr) => attr.clone());
+        return new DeviceSelector(this.kind, this.id, this.principal, attributes);
+    }
+
+    toString() {
+        return `Device(${this.kind}, ${this.id ? this.id : ''}, )`;
+    }
+}
+DeviceSelector.prototype.isDevice = true;
+Selector.Device = DeviceSelector;
+
+
+class BuiltinDevice extends Selector {
+    clone() {
+        return new BuiltinDevice();
+    }
+
+    toString() {
+        return 'Builtin';
+    }
+}
+BuiltinDevice.prototype.isBuiltin = true;
+
+/**
+ * A selector that maps the builtin `notify`, `return` and `save` functions.
+ *
+ * This is a singleton, not a class.
+ *
+ * @alias Ast.Selector.Builtin
+ * @readonly
+ */
+Selector.Builtin = new BuiltinDevice();
+module.exports.Selector = Selector;
 
 /**
  * An invocation of a ThingTalk function.

lib/ast/slots.js

@@ -34,6 +34,7 @@ class AbstractSlot {
      * @protected
      */
     constructor(prim, scope) {
+        assert(prim || prim === null);
         this._prim = prim;
 
 
@@ -144,6 +145,37 @@ class InputParamSlot extends AbstractSlot {
     }
 }
 
+class DeviceAttributeSlot extends AbstractSlot {
+    constructor(prim, attr) {
+        super(prim, {});
+        this._slot = attr;
+        assert(this._slot.name === 'name');
+    }
+
+    toString() {
+        return `DeviceAttributeSlot(${this._slot.name} : ${this.type})`;
+    }
+
+    get type() {
+        return Type.String;
+    }
+    get tag() {
+        return `attribute.${this._slot.name}`;
+    }
+    getPrompt(locale) {
+        // this method should never be used, because $? does not typecheck in a device
+        // attribute, but we include for completeness, and just in case
+        const gettext = I18n.get(locale);
+        return gettext.dgettext('thingtalk', "Please tell me the name of the device you would like to use.");
+    }
+    get() {
+        return this._slot.value;
+    }
+    set(value) {
+        this._slot.value = value;
+    }
+}
+
 class FilterSlot extends AbstractSlot {
     constructor(prim, scope, arg, filter) {
         super(prim && prim.isPermissionRule ? null : prim, scope);
@@ -269,7 +301,7 @@ class ArrayIndexSlot extends AbstractSlot {
         return this._type;
     }
     get tag() {
-        return `${this._baseTag}[${this._index}]`;
+        return `${this._baseTag}.${this._index}`;
     }
     getPrompt(locale) {
         const gettext = I18n.get(locale);
@@ -456,7 +488,8 @@ module.exports = {
     recursiveYieldArraySlots,
     makeScope,
     InputParamSlot,
+    DeviceAttributeSlot,
     FilterSlot,
     ArrayIndexSlot,
-    FieldSlot
+    FieldSlot,
 };

lib/compiler/ops-to-jsir.js

@@ -48,10 +48,17 @@ module.exports = class OpCompiler {
             ast.__effectiveSelector = ast.selector;
         }
 
-        // TODO more attributes, and dynamic attributes (param-passing)
         const attributes = {};
         if (ast.__effectiveSelector.id)
             attributes.id = ast.__effectiveSelector.id;
+        // NOTE: "all" has no effect on the compiler, it only affects the dialog agent
+        // whether it should slot-fill id or not
+        // (in the future, this should probably be represented as id=$? like everywhere else...)
+
+        for (let attr of ast.__effectiveSelector.attributes) {
+            // attr.value cannot be a parameter passing in a program, so it's safe to call toJS here
+            attributes[attr.name] = attr.value.toJS();
+        }
         return [ast.__effectiveSelector.kind, attributes, ast.channel];
     }
 

lib/describe.js

@@ -13,7 +13,7 @@ const assert = require('assert');
 
 const Ast = require('./ast');
 const Type = require('./type');
-const { clean } = require('./utils');
+const { clean, cleanKind } = require('./utils');
 const FormatUtils = require('./format_utils');
 const { Currency } = require('./builtin/values');
 const I18n = require('./i18n');
@@ -351,6 +351,14 @@ class Describer {
         return recursiveHelper(expr);
     }
 
+    _getDeviceAttribute(selector, name) {
+        for (let attr of selector.attributes) {
+            if (attr.name === name)
+                return this.describeArg(attr.value, {});
+        }
+        return undefined;
+    }
+
     describePrimitive(obj, scope, extraInParams = []) {
         if (obj.selector.isBuiltin) {
             if (obj.channel === 'return')
@@ -378,10 +386,26 @@ class Describer {
                 throw TypeError('Invalid @remote channel ' + channel);
         } else {
             confirm = schema.confirmation;
+
+            let cleanKind = obj.schema.class ? obj.schema.class.canonical : clean(obj.selector.kind);
+
+            let selector;
+            let name = this._getDeviceAttribute(obj.selector, 'name');
             if (obj.selector.device)
-                confirm = confirm.replace('$__device', obj.selector.device.name);
+                selector = this._("your %s").format(obj.selector.device.name);
+            else if (obj.selector.all && name)
+                selector = this._("all your %s %s").format(name, cleanKind);
+            else if (obj.selector.all)
+                selector = this._("all your %s").format(cleanKind);
+            else if (name)
+                selector = this._("your %s %s").format(name, cleanKind);
             else
-                confirm = confirm.replace('$__device', clean(kind));
+                selector = this._("your %s").format(cleanKind);
+
+            if (confirm.indexOf('$__device') >= 0)
+                confirm = confirm.replace('$__device', selector);
+            else if (confirm.indexOf('${__device}') >= 0)
+                confirm = confirm.replace('${__device}', selector);
         }
 
         let firstExtra = true;
@@ -832,9 +856,9 @@ class Describer {
 
             switch (functionType) {
             case 'query':
-                return this._("your %s").format(doCapitalizeSelector(kind));
+                return this._("your %s").format(capitalize(cleanKind(kind)));
             case 'action':
-                return this._("perform any action on your %s").format(doCapitalizeSelector(kind));
+                return this._("perform any action on your %s").format(capitalize(cleanKind(kind)));
             default:
                 return '';
             }
@@ -957,7 +981,7 @@ class Describer {
 }
 
 function capitalize(str) {
-    return (str[0].toUpperCase() + str.substr(1)).replace(/[.\-_]([a-z])/g, (whole, char) => ' ' + char.toUpperCase()).replace(/[.\-_]/g, '');
+    return str.split(/\s+/g).map((word) => word[0].toUpperCase() + word.substring(1)).join(' ');
 }
 
 function capitalizeSelector(prim) {
@@ -970,27 +994,10 @@ function capitalizeSelector(prim) {
 }
 
 function doCapitalizeSelector(kind, channel) {
-    // thingengine.phone -> phone
-    if (kind.startsWith('org.thingpedia.builtin.thingengine.'))
-        kind = kind.substr('org.thingpedia.builtin.thingengine.'.length);
-    // org.thingpedia.builtin.omlet -> omlet
-    if (kind.startsWith('org.thingpedia.builtin.'))
-        kind = kind.substr('org.thingpedia.builtin.'.length);
-    // org.thingpedia.weather -> weather
-    if (kind.startsWith('org.thingpedia.'))
-        kind = kind.substr('org.thingpedia.'.length);
-    // com.xkcd -> xkcd
-    if (kind.startsWith('com.'))
-        kind = kind.substr('com.'.length);
-    if (kind.startsWith('gov.'))
-        kind = kind.substr('gov.'.length);
-    if (kind.startsWith('org.'))
-        kind = kind.substr('org.'.length);
-    if (kind.startsWith('uk.co.'))
-        kind = kind.substr('uk.co.'.length);
+    kind = cleanKind(kind);
 
     if (kind === 'builtin' || kind === 'remote' || kind.startsWith('__dyn_'))
-        return capitalize(channel);
+        return capitalize(clean(channel));
     else
         return capitalize(kind);
 }