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

Extend device selectors to refer to devices by name #162

Merged
merged 14 commits into from
Nov 1, 2019

Conversation

gcampax
Copy link
Contributor

@gcampax gcampax commented Nov 1, 2019

So we can ask "turn on my bedroom lights"

This is a feature asked by @balloob.
This is only part 1. Part 2 is the dialog agent to actually select the device. Part 3 is Genie to train those sentences. Part 4 is every Thingpedia device that would like this functionality should add relevant templates and change their confirmation string.

Attributes are key-value pairs that allow the user to select a device by
name, location, or other characteristic.

Currently, the only allowed attribute is name, of String type.

The AST is more flexible though to support parameter passing (needed
to make primitive templates with device names). $? is not supported
because it's not very useful - Almond natively asks for which
device anyway.
"all" is a boolean; if set, device selection is skipped and the program
operates on all devices that match the selector
(unset is the same as false)
This allows Genie to do beta-reduction of device attributes
in primitive templates
Improve on our existing $__device support.

Next step: changing every single Thingpedia device to support this...
To test some parts of the code where, if we fail, we only find out
in almond-dialog-agent or genie and then it's pain to fix
@gcampax gcampax requested a review from sileix November 1, 2019 07:01
@gcampax gcampax added this to In Progress in Home Assistant via automation Nov 1, 2019
@coveralls
Copy link

coveralls commented Nov 1, 2019

Pull Request Test Coverage Report for Build 718

  • 121 of 140 (86.43%) changed or added relevant lines in 11 files are covered.
  • 42 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.01%) to 82.173%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/ast/slots.js 11 12 91.67%
lib/describe.js 22 24 91.67%
lib/prettyprint.js 10 12 83.33%
lib/utils.js 13 16 81.25%
lib/typecheck.js 24 28 85.71%
lib/nn-syntax/tonn_converter.js 9 16 56.25%
Files with Coverage Reduction New Missed Lines %
lib/nn-syntax/parser.js 42 68.86%
Totals Coverage Status
Change from base Build 713: -0.01%
Covered Lines: 7162
Relevant Lines: 8478

💛 - Coveralls

So the code coverage does not drop too much.

At the same time, it's clear the heuristic used is pretty gross,
and we should use the #_[canonical] annotation on the class instead.
Because almond-dialog-agent indirectly does that when typechecking
predicates on their own
Otherwise it is annoying to parse the tag to extract the argument
name during parameter replacement
Otherwise we cannot beta-reduce parameters in them during Genie
generation
Copy link
Member

@sileix sileix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

}

toString() {
return `Device(${this.kind}, ${this.id ? this.id : ''}, )`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no attributes?

@gcampax gcampax merged commit 917b255 into master Nov 1, 2019
Home Assistant automation moved this from In Progress to Done Nov 1, 2019
@gcampax gcampax deleted the wip/device-selector branch November 1, 2019 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants