things: Samsung Smart Airconditioner support

[CloCkWeRX]

Follows on from #100

We've got a library, discovery, rough API in isolation from steward - we should look to wire it up properly as a thing; to the point it can appear in the d3 client.

Blocked a little bit by not having a test/fake server https://github.com/TheThingSystem/node-samsung-airconditioner/issues/1

Additionally, a few SVG assets for the d3 client wouldn't hurt.

[mrose17]

Congrats! I will get back with a substantive reply in a couple of hours...

[mrose17]

"get the latest", then do this:

    % cd steward
    % cp drivers/climate-control-template.js steward/devices/devices-climate/climate-samsung-control.js
    % cd steward

then edit package.json to include a pointer to your module, e.g., under "dependencies" add

    , "samsung-airconditioner": "https://github.com/CloCkWeRX/node-samsung-airconditioner.git"

then do

    % npm -l install

then edit devices/devices-climate/climate-samsung-control.js - look for every occurrence of TBD and perform the needful. then do this:

    % ../scripts/jshint.js

and if it complains about anything in climate-samsung-control.js … fix it!

at this point you can launch the steward by hand:

    # sh /etc/init.d/steward stop
    # ./run.sh

this will put the output on your terminal so you can see what's going on.

good luck!

[CloCkWeRX]

Is there much of a persistence layer that I can work with? Have discovery happening, but wondering where to persist the token after the first setup.

[CloCkWeRX]

Well, that makes it more or less work.

Problems:

• Have had to hardcode the token for now
• The UI doesn't really fit well with the capabilities of the machine - not all features are mapped.

[mrose17]

first, congrats on getting this far! seriously, it's a big system, lots of moving parts, etc...

problem 1 - oops, should have told you how to do that! look at devices/devices-indicator/indicator-wink-dial.js - you'll see it calls two routines

    self.getState(function(err, state))

and

    self.setState(state) 

that it inherits from the root device prototype.

this is what you want to use to store the token. as always, i suggest instead of storing a string, story a property list (javascript object), e.g.,

    self.stateState({ token: '…' });

that makes extensibility easier.

as to the second issue… this may be "too much information" (-; but here we go:

this is actually a philosophical bias in the steward: many years ago i was involved in the SNMP effort (some folks have noted that #thethingsystem is basically SNMP for automation, and i would only partially disagree). one of the things that we learned in that effort was the importance of finding the right balance between the abstract model for a device type (router, bridge, modem, etc.) and the real world devices offered by vendors. in SNMP, the process to find the right balance was done in relatively small committees managed by a small group of SNMP gurus. it worked out pretty good (never optimal, but rarely awful).

for the #thethingsystem, it's the curators who define the taxonomy which is roughly equivalent to the abstract model. we don't claim to be particularly clever, so we use the "smart small and add later" philosophy, e.g., for an HVAC system, what are the smallest set of things that a consumer would use. do that initially and make sure that the UI support that taxonomy. see how that goes and iterate.

i agree that the HVAC taxonomy, which the HTML5/D3 client implements isn't really optimal for the samsung air conditioners. philosophically, i wonder whether we should have a similar abstract model, but tailored more for fans. or maybe we should just figure out the smallest thing to add for fans. i'm not sure yet.

however, if you can humor me by implementing a steward driver that does the HVAC abstract model using node-samsung-airconditioner, that's the first step…

many thanks!

[CloCkWeRX]

this is what you want to use to store the token. as always, i suggest instead of storing a string, story a property list (javascript object), e.g.,

self.stateState({ token: '…' });

OK, will do. I still have the problem of 'prompt the user to go do an arbitrary task at a point in time to complete auth' - any suggestions?

it's the curators who define the taxonomy which is roughly equivalent to the abstract model. we don't claim to be particularly clever, so we use the "smart small and add later" philosophy, e.g., for an HVAC system, what are the smallest set of things that a consumer would use. do that initially and make sure that the UI support that taxonomy. see how that goes and iterate.

I agree with the overall approach. It might be really hard to pull it off, but UI by composition of atomic components might be a better long term metaphor: as you said, a HVAC is really just fan speed, temperate control, mode (heat, cool, fan, off, auto) at its heart.

The presence of those features in a unit just makes the actual aircon product itself a cardboard box used by marketing people to glue all of those together on a shelf to the point they get a paycheck - and since a lot of other people do a lot of other combinations, there's an incentive to introduce non standard modes as a point of difference to get more paychecks.

(I say this without having dug into the code for the d3 client yet)

Given that, if it were trivial to compose a UI as something that is "temperature settable", "fan-mode-settable", etc and introduce "proprietary-foo-changeable"; that'd be quite neat. Ember.js for example enforces a good separation between template and behaviour for a lot of HTML based components; and allows you to include behaviours via mixins - perhaps a similar pattern can be introduced here.

however, if you can humor me by implementing a steward driver that does the HVAC abstract model using node-samsung-airconditioner, that's the first step…

So, I've done what I can as of 216f9f91bd8266875544608466af7bff7ca32272 ; barring get_temperature/current device state. I'll probably need to think more about reacting to physical updates to the A/C and if it pushes to us.

[aallan]

Of course the other philosophical point is that we don't view the current d3 client as "the client".

Like Twitter in the early days we're hoping that people will come in and build clients and evolve the UX—possibly in ways we never thought about. Unlike Twitter we don't intend to cut them off at the knees after they've done that.

[mrose17]

@aallan - thanks. i left that out and i shouldn't have. the HTML5/D3 client is the "pro to client" -- we knew that there had to be at least one client out the door, that it had to have good functionality, and it had to be visual. so, i set out some guidelines and then guided the development effort.

[mrose17]

@CloCkWeRX - sorry, here's how you tell folks about that: take a look at devices/devices-ilghting/lighting-hue.js

in the object constructor, you'll see this:

    if (request === 'attention') {
      if (self.status === 'reset') self.alert('please push pairing button on Hue bridge');
      return;
    }

what that says is:

1. when a new client connects, an 'attention' request is published to all devices, which asks: anything we need to tell the user about?
2. the device (in this case the philips hue controller) looks at it's state. a state of 'reset' means "i need external help to configure", so if self.status is 'reset', it calls self.alert('…'); which the steward will then forward to all connected clients.

when you're happy with that, can you give me a link to the whole file, so i can give it a quick once over? thanks!

[mrose17]

@CloCkWeRX - hi. how are things looking on this?

[CloCkWeRX]

Distracted by day job :/

[mrose17]

thanks… no worries, just curious!

[CloCkWeRX]

For some reason after a while (probably the aircon severing the connection); I get this

uncaught exception: TypeError: Cannot call method 'indexOf' of undefined

[{
    "fileName": "/home/pi/steward/steward/core/utility.js",
    "lineNumber": 101,
    "functionName": "logger.log",
    "typeName": "logger",
    "methodName": "log",
    "columnNumber": 13,
    "native": false
}, {
    "fileName": "function) [as info] (/home/pi/steward/steward/node_modules/winston/lib/winston/common.js",
    "lineNumber": 45,
    "functionName": "target.(anonymous",
    "typeName": "target",
    "methodName": "(anonymous",
    "columnNumber": 21,
    "native": false
}, {
    "fileName": "/home/pi/steward/steward/devices/devices-climate/climate-samsung-control.js",
    "lineNumber": 249,
    "functionName": null,
    "typeName": null,
    "methodName": null,
    "columnNumber": 15,
    "native": false
}, {
    "fileName": "/home/pi/steward/steward/node_modules/samsung-airconditioner/samsung-airconditioner.js",
    "lineNumber": 105,
    "functionName": "null._onTimeout",
    "typeName": "null",
    "methodName": "_onTimeout",
    "columnNumber": 27,
    "native": false
}, {
    "fileName": "[as ontimeout] (timers.js",
    "lineNumber": 110,
    "functionName": "Timer.listOnTimeout",
    "typeName": "Timer",
    "methodName": "listOnTimeout",
    "columnNumber": 15,
    "native": false
}]

[CloCkWeRX]

Second bit of trouble I'm running into - I need to call getState()/setState() from outside of the object.

Its not entirely clear to me how best to do it, beyond

  function read_token(deviceUID) {
    var token = null;
    db.get('SELECT value FROM deviceProps WHERE deviceUID=$deviceUID AND name="token"', { $deviceUID: deviceUID }, function(err, row) {
      if (!!row) {
        token = row.value;
      }
    });

    return token;
  }

  function save_token(deviceUID, token) {
    db.get('INSERT INTO deviceProps(deviceID, key, value) VALUES($deviceID, "token", $value)', { $deviceUID: deviceUID, $value: token });
  }  

and

devices.discover(info, function (err, deviceID) {
      var token = read_token(info.id);

     if (!token) { get_token(); save_token(info.id, ''bar') login(token); } else { login(token); }
}

[mrose17]

i'm afraid that approach won't work... you need to call getState/setState within the object. the database is for devices that have been constructed.

in the constructor, you should call getState. if there isn't a token, then you should set the thing's state to 'reset' so that when the steward probes for devices needing attention, your code can generate a response. whenever it generates a response, then it should call the code that gets the token, which ultimately can call setState and change the status to something other than 'reset'.

does that make sense?

/mtr

ps: lighting-hue follows a similar approach, although it doesn't wait for an attention to be sent out, it just goes into a retry loop. i'm not sure which approach is better.

[mrose17]

regarding:

    uncaught exception: TypeError: Cannot call method 'indexOf' of undefined

the likely cause is the logging statement itself. what is at this line:

    "fileName": "/home/pi/steward/steward/devices/devices-climate/climate-samsung-control.js",
    "lineNumber": 249,
    "functionName": null,
    "typeName": null,
    "methodName": null,
    "columnNumber": 15,
    "native": false

thanks!

[CloCkWeRX]

i'm afraid that approach won't work... you need to call getState/setState within the object. the database is for devices that have been constructed.

Yeah, I could see that, but still had a bit of trouble getting to where I wanted.

Ie, I'm currently doing:

    info.url = info.device.url;
    info.deviceType = '/device/climate/samsung/control';
    info.id = info.device.unit.udn;
    if (!!devices.devices[info.id]) return;

    // TODO: This needs to be either setup || load from DB if known, and save/read data from the store.

    token = '98854465-6273-M559-N887-373832354144';

    aircon.login(token, function () {
      logger2.info(info.device.name, info.device);
      devices.discover(info);
    });

I tried to do

    devices.discover(info, function (err, deviceID) {
      var token = read_token(info.id); 
      if (!token) {
        aircon.get_token(function(err, token) {
          if (!!err) return console.log('get_token error: ' + err.message);

          console.log('Token is ' + token);

          save_token(info.id, token);

          aircon.login(token, function () {
            logger2.info(info.device.name, info.device);
          });
        }).on('waiting', function() {
          devices.devices[info.id].alert('Please power on the device within the next 30 seconds');
        });        
      } else {

        aircon.login(state.token, function () {
          logger2.info(info.device.name, info.device);
        });
      }

read_token - I would have called this on the actual device instance - devices.devices[info.id].device.getState().token or similar, but the instantiated device was always undefined.

The best I could figure, in the core device API

exports.discover = function(info, callback) {

     ....

      if (!!callback) callback(null, null); // Always happens before instantiation, no other callbacks executed

      if (!info.device.name) info.device.name = 'device/' +  row.deviceID;
      devices[deviceUID].device = new (makers[deviceType])(row.deviceID, deviceUID, info); // This is the actual instance I want to work with.

[CloCkWeRX]

Had a a fresh look. I manually had to drop into SQLite, because I was doing

      devices.devices[deviceID].device.setState({token: '98854465-6273-M559-N887-373832354144'});
      console.log(devices.devices[deviceID].device.getState());

and getting undefined.

For some reason, I couldn't write to the DB and was getting silent failures

Device.prototype.setState = function(state) {

    /// This failed
    db.run('REPLACE INTO deviceProps(deviceID, key, value) VALUES($deviceID, $key, $value)',
           { $deviceID : self.deviceID, $key: 'state', $value: JSON.stringify(state) });

Manually doing it yielded:

sqlite> REPLACE INTO deviceProps(deviceID, key, value) VALUES(3, 'state', '{"token":"98854465-6273-M559-N887-373832354144"}');
Error: attempt to write a readonly database

... and I realized I probably wanted to do this via sudo, which worked.

Is the DB write failing a bug you reckon, or I've trashed my setup in some way?

[mrose17]

i don't know about the DB.

i do know that the approach your taking is problematic. is there some reason why you can not call getState inside the constructor. the software is organized to make that easy…

perhaps you might put your driver file in a gist and i can take a look at it...

[CloCkWeRX]

i do know that the approach your taking is problematic. is there some reason why you can not call getState inside the constructor. the software is organized to make that easy…

I have a strong aversion to work in the constructor (http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/) - violates SRP, configuration of the objects and the instance of the objects should be loosely coupled, etc etc etc. I also find there's a lot of magic with global state going on, which makes it confusing if you come at it from the context of writing a single device. Not a problem for you, you know the whole stack fairly well, hurts my brain a bit though.

Couple that with nodejs itself keeps brutally reminding me that "asynchronously, it's messages, totally, are" by breaking code in what I find to be surprising ways (and a node guru would not).

Lets not get into that kind of stuff, because only working code matters :)

[mrose17]

i don't disagree with your analysis on both the nature of constructors or having to understand the whole stack to write a driver. let me take a look at the PR and see where we're at...

[CloCkWeRX]

Status:

So it 'works', but some of the wiring to the ui isn't in place. For example, I hooked up the fan on/off/auto in the api, and heat/cool/etc, but the controls remain inert.

No on/off appears for the ui either, though - for regular thermostats it might not make sense - not the same way it as it does for a wall mounted a/c with thermostat.

I assume the thermostat ui component is either not yet fully implemented or I have miss configured something slightly.

I need to have the lower level library broadcast events for manually triggered state change.

I need to store the current temp in the state of the device.

[mrose17]

thanks. ok, here's what i suggest:

1. invoke setup in the constructor. you can invoke it in the callback from devices.discover(), but none of the other 63 device drivers are coded that way.

2. fyi: whenever you are writing a callback where the first argument is 'err' or 'e' or 'error', etc., you almost always want the first procedural line in the callback to be something like this:

   if (!!err) { … };

or

    if (!err) return …;

the '…' depends on each callback, of course. but the rule is always check for the error first and usually return from the callback early if there was an error.

1. if you look at the $info.properties object, you'll see the properties that the device is supposed to support. 'status' and 'name' are special, everything else goes into self.info. in the template you started with, the update() method is supposed to set self.info - so here's the big question, how do you find out the current temperature, humidity, mode, and goal temperature for the air conditioner?

if you get this via an event, then you can listen for the event and when it comes in, you invoke the update() method. if you have to poll, then we can poll for that. i see a get_temperature. is there a way to get humidity, and current operating mode?

1. when logging, the first argument should almost always be

   'device/' + self.deviceID

and the logging subsystem will fill-in the name for you.

1. since this is an air conditioner, you probably want to remove 'heat' from the hvac choices.

2. you also need to add status after name, my guess is that you want:

   status: [ 'present', 'reset' ]

'reset' is the state you put it into when you don't have a token.

7, i will try to get the F/C fixes to the HTML5/D3 client prioritized up. until then instead of

    http://127.0.0.1:8887/

use

    http://127.0.0.1:8887/console

this tells you how the steward sees things and is more useful during development.

finally, sorry this is so painful. the good news its hat you're close to being done.

thanks!

[CloCkWeRX]

• so here's the big question, how do you find out the current temperature, humidity, mode, and goal temperature for the air conditioner?

It'll be events from the lower level library which are not yet being issued (the xml is certainly appearing, but unrecognized)

since this is an air conditioner, you probably want to remove 'heat' from the hvac choices.

Interestingly the a/c has an explicit heat mode. Most of the time though, 'auto' decides which mechanism to use I'm guessing.

invoke setup in the constructor. you can invoke it in the callback from devices.discover(), but none of the other 63 device drivers are coded that way.

Convention wins then, fixed.

[mrose17]

thanks for giving me that one (-;

leave the 'heat' then. that is sort of odd, i have to admit.

i think the key is to get events passed up to the driver so it can call the update method(), at that point, i think it all falls into place (-;

[CloCkWeRX]

Can I get you to git pull on node-samsung-airconditioner? added a few docs of the type of events it emits

[mrose17]

done. are changes being made to node-samsung-airconditioner? if so, pleas elet me know.

[CloCkWeRX]

So very close, but I have to give it in for the night!

Will share this with you though:

As well as this news article

http://www.abc.net.au/news/2014-01-16/heatwave-brings-record-temperatures-blackouts-and-health-dangers/5202886

You cannot imagine how much I appreciate it that you've been helping with this darned airconditioner.

[mrose17]

@CloCkWeRX - could you run console and get me a screen shot of what it says about your unit, here's a screen shot of mine. try to stay cool!

[CloCkWeRX]

Initial state, because we need to do a one-off poll()/scan()

after playing with buttons on the unit, it was populating content as expected

Unfortunately I need to tweak some of the wiring - for example, switching the fan off and going into cool mode switches everything off, because I didn't quote understand the events.

Will be fixing a bit over the next hour or two.

[mrose17]

i think you uploaded the same image twice! (-;

[CloCkWeRX]

See the edit

On Sun, Jan 19, 2014 at 3:42 PM, mrose17 notifications@github.com wrote:

i think you uploaded the same image twice! (-;

— Reply to this email directly or view it on GitHubhttps://github.com/TheThingSystem/steward/issues/113#issuecomment-32701336 .

[mrose17]

thanks!

[CloCkWeRX]

TODO:

The UI doesn't update after you issue a command yet, as we aren't keeping a track of the command ids issued or updating the internal state when we try to issue one.

ie:

<Request Type="DeviceControl" CommandID="cmd3616">
   <DoSomething />
</Request>

gets

<?xml version="1.0" encoding="utf-8" ?><Response Type="DeviceControl" Status="Okay" DUID="7825AD103D06" CommandID="cmd3616"/>

TODO: D3 client, hvac UI would be good to automatically add renderings for modes (only Off/Fan/Cool/Heat appear, no Auto)

TODO: There are 'fan levels' (low, medium, high), which aren't really supported by the UI.

[CloCkWeRX]

JSHint done.

[mrose17]

for your next revision of climate-samsung-control.js: when you create the info object prior to calling devices.discover, the 'url' field is set to null. please set it to something like 'tcp://IPADDR:PORTNO' -- it turns out that it's useful for the device module to know the ip address… thanks!

[mrose17]

@CloCkWeRX - status?

[CloCkWeRX]

Some minor bugs with current state detection. Have paused for day job for a bit. GoalTemp started working in latest master though ! On 23/01/2014 7:41 PM, "mrose17" notifications@github.com wrote:

@CloCkWeRX https://github.com/CloCkWeRX - status?

— Reply to this email directly or view it on GitHubhttps://github.com/TheThingSystem/steward/issues/113#issuecomment-33107438 .

[mrose17]

groovy! keep at it when you have the time, thanks!

[CloCkWeRX]

Things are a bit better now. I've put in a little bit of work into the d3 client, but its a bit hacky.

You can see there's some kind of bug with initializing the settings for the UI - metric is saved, but doesn't take effect unless I interact with the content and 'save' it again.

For wall mounted air conditioners, I've added an explicit 'power', plus on/off toggle control. I realise I should use status, like many of the other switches, but since we're already making use of that for presence/reset/etc in the samsung case, I could use a little advice.

I've also added an Auto mode, for when you are interested in letting the device decide if things should heat / cool.

What I'm really lacking is a way to inspect the properties of the device - I've already published that I support these things, but the UI doesn't seem to have much awareness of them.

  steward.actors.device.climate.samsung.control =
      { $info     : { type       : '/device/climate/samsung/control'
                    , observe    : [ ]
                    , perform    : [ ]
                    , properties : { name            : true
                                   , status          : [ 'present', 'reset' ]
                                   , lastSample      : 'timestamp'
                                   , temperature     : 'celsius'
                                   , humidity        : 'percentage'
                                   , power           : [ 'on', 'off' ]
                                   , hvac            : [ 'cool', 'heat', 'fan', 'dry', 'off', 'auto', ]
                                   , fan             : [ 'on', 'off', 'high', 'mid', 'low', 'auto', 'milliseconds' ]
                                   , goalTemperature : 'celsius'
                                   }
                    }
        , $validate : { perform    : validate_perform }
      };

It's feeling a bit unlike the other thermostat models - the commonality is there, but because of the mix of thermostat, hvac, fan, power (on/off vs continual supply) and other components; it doesn't all gel together.

[CloCkWeRX]

Still on the todo:

• Fan time (aka sleep mode?) - it's sort of implemented in the backend but not wired up
• It's common to set fan intensity, possibly a UI control for it is worthwhile (every fan I have encountered has some variation on this, inside of an air conditioner or not)
• Consider an alternative visualisation of the thermostat knob, add units to it - it's really unclear that it represents "current temp" and dominates a lot of screen real estate. Perhaps just rendering figures (kind of like the google weather controls) would be more informative.

[CloCkWeRX]

Oh, and the temperate slider could really use a hint to know if its an analog range or digital range - ie my unit only does steps of 1.0 degrees C. It'd be interesting to try out a html5 slider input, if support is widely available; as it adds min/max/step/etc.

[mrose17]

where do things stand now? is it feasible to add this to the steward? thanks!

[CloCkWeRX]

I think so. Depends on scrutiny of ui changes vs bigger picture. Minor ui bugs with localisation not kicking in always, and on off ui being sluggish to respond. For me, I find it usable On 04/02/2014 4:53 PM, "mrose17" notifications@github.com wrote:

where do things stand now? is it feasible to add this to the steward? thanks!

Reply to this email directly or view it on GitHubhttps://github.com/TheThingSystem/steward/issues/113#issuecomment-34034004 .

[CloCkWeRX]

Going to close this if favour of #144 and and rework needed there. Will raise bug reports for the localization/etc after merging.