GPII-413: Add a dynamic Device Reporter

[javihernandez]

Related literature about this can be found here, and can be tested with this pull request to GPII/linux.

The implementation follows a similar approach to how the settingsHandlers work.
Here you can see how a solution entry should be written in order to call to a deviceReporter plugin.

{  
    "name": "ORCA Screen Reader",
    "id": "org.gnome.orca",
    "contexts": {
    "OS": [{
        "id": "linux",
        "version": ">=2.6.26"
    }],
    "isInstalled": [
        { 
            "type": "gpii.packageKit.find",
            "name": "orca"
        }
    ]
}

For this first example I have used the packageKit module for GNU/Linux but we could create a few common deviceReporter plugins such as:

• A DR plugin that could look for a file in the filesystem, ie, look for /usr/opt/bin/google-chrome
• A DR plugin that could check if a process is running on the system, ie: firefox

Inspired by this, and as a replacement for the Linux' packageKit module in Windows, a node module can be built in order to poke the Windows registry for installed solutions.

Any testing/feedback will be awesome :)
Cheers!

[amb26]

Use === for comparison here - we should ensure that these files are lintable using jshint and, for example, the infusion .jshintrc - also there is a missing semicolon on this line

[sgithens]

I've added a pull request to Javi's branch at javihernandez#6
This is based on some work Antranig and I did during a break at the HCII conference. It allows the entire device Reporter section to be passed in and the argumentMapping to actually work. This is needed because we will need more than just the 'name' property on the device reporter json section for other device reporter plugins. (Such as the windows registry device reporter)

[sgithens]

@javihernandez @amb26 Just pinging to see if there is any more discussion on this ticket, and how much more engineering is required for it. Is it just in holding until the unit tests for the linux side of the pull request are updated based on that pull request?

[sgithens]

Just checking in again to see if there is anymore work to be done on this specific pull request, or if it's just waiting for finishing touches on the linux side.

[javihernandez]

@sgithens, still working on the linux side of the pull request after the latests comments from Antranig.
In this direction, we've already added javihernandez/linux@90e17d3 to that pull request and we (Joseph and I) are working on the last comment from Antranig here javihernandez/linux#11

I'll ping Antranig once that pull request is ready for another review.

[sgithens]

@javihernandez Thanks for the info Javi!

[amb26]

Hi - I saw these comments in an email notification but I don't seem to see them in the pull here:

@amb26, splitted into 'static' and 'live' here, but I still need to address the "Other notes".

ii) how can I use the flowManager's solutionsRegistryDataSource? By having replaced the current {solutionsRegistryDataSource} with {flowManager}.solutionsRegistryDataSource in my argument list didn't do the trick?
iii) How? Should I wrap this into a callback for deviceReporter.get? Or should I add a new invoker in gpii.deviceReporter.base?

ii) how "didn't do the trick"? It should work fine as far as I can see. What result do you get?
iii) A new invoker, I think

[javihernandez]

@amb26,

ii) I'm getting an empty response from the solutions registry, so when calling to localhost:8081/device I'm getting

{"solutions":[],"OS":{"id":"linux","version":"3.11.10-21-desktop"}}

iii) I did this in javihernandez@f842a40, is this correct?

[amb26]

ii) That seems very odd - could you check in a branch that shows this behaviour and tell me how to trigger it? We should make sure that this is working

iii) looks good, yes

[javihernandez]

@amb26, wrt ii), I've just pushed it here, and the commit that makes the change is this one.

[klown]

@javihernandez I'm not sure if this addresses the issue, but there is a problem in DeviceGet.js. There is a "func" property in the invoker section, but it should be "funcName". See:

https://github.com/javihernandez/universal/blob/device-reporter-draft-2/gpii/node_modules/deviceReporter/src/DeviceGet.js#L28

[amb26]

@klown - "func" is correct here, although with the current framework there is no difference.
@javihernandez - I tried out your "tempBranch" and it works fine - a call to localhost:8080/device produces the following output:
http://pastie.org/10208472

Perhaps you could put a log statement in line 314 of Kettle's dataSource.js to read the following, and see if it helps to see what is happening:

console.log("Calling ", func, " for dataSource get of url " + url);

[klown]

@javihernandez, @amb26, I recently pushed a version of the ProcessReporter[1] that uses grades instead of the fat API, and it works in both development and production configurations. I decided to come back to the device reporter and see if I could detect a difference to explain your problem @javihernandez . But, when I run this "draft2" branch of the deviceReporter in development and production configurations, I get what is expected:
http://pastie.org/10224046 -- look for headings "Development" and "Production".

[1] #355, specifically klown@8381ec7

[klown]

@javihernandez, @amb26, never mind me. I just read the comment about duplicating "the entire solutionsRegistryDataSource", and realized that's what the issue is. I'll look into it since the ProcessReporter is likewise duplicating.

[klown]

@javihernandez , @amb26, When I test Javi's "tempBranch", I get the same result as he does. It depends on running with the development configuration, i.e. using all.development.dr.production.json

Adding a "console.log()" as @amb26 suggested gives the following. The problem is the url to the test data ends in "null.json" instead of "linux.json". Trying to figure out why:

Calling  function (fileName, options, data) {
    var promise = fluid.promise(),
        method = "readFile",
        fsCallback = function (error, data) {
            if (error) {
                promise.reject({
                    isError: true,
                    message: error.message
                });
            } else {
                promise.resolve(options.operation === "set" ? undefined : data);
            }
        },
        args = [fileName, "utf8"];
    if (options.operation === "set") {
        method = "writeFile";
        args.splice(1, 0, data);
    } else {
        if (!fs.existsSync(fileName)) {
            promise.resolve(undefined); // dataSource semantics - read from nonexistent file produces undefined
            return;
        }
    }
    args.push(kettle.wrapCallback(fsCallback));
    fs[method].apply(null, args);
    return promise;
}  for dataSource get of url /home/clown/Development/GPII/DeviceReporter/node_modules/universal/gpii/node_modules/flowManager/../../../testData/solutions/null.json

[klown]

@javihernandez, @amb26,

I wrote:

The problem is the url to the test data ends in "null.json" instead of "linux.json". Trying to figure out why:

The first argument to solutionsRegistryDataSource() at line 100 is 'null'. But it needs to communicate the OS somehow.

I don't know how to do that. To test the hypothesis, I tried:

var pltfrm = { os: deviceReporter.platformReporter.reportPlatform().id };
solutionsRegistryDataSource.get(pltfrm, function onSuccess(entries) { ...

And that worked. But, there's got to be a better way.

[javihernandez]

@klown,

that's correct, I was missing that. Thanks a lot!!
I'm updating it now.

[javihernandez]

up

[amb26]

If a 2nd onCreate fires, one should imagine that it is part of a further test case which may well require different expectInstalled contents. So we should just remove this branch.

[javihernandez]

This is how I'm checking whether we need to mock or not. Isn't it the right place/way for doing this?

[amb26]

The nameResolver should give a completely fresh name for the deviceReporter under the mock configuration - for example, the way our settingsHandlers do. There should never be a question of overwriting the global name of the real deviceReporter, etc.

[amb26]

http://www.w3.org/Provider/Style/URI.html

[javihernandez]

@amb26

I've updated this, but I'm not feeling specially proud of it. Can you take a look at it?

[amb26]

This method is ok but messy - it will be clearer for readers (and for YOU, VANTTUNEZZ!) if you just use the same scheme that we have for settings handlers - that is, just inject the nameResolver as a component of gpii.deviceReporter.base and expect for all users to invoke it from there (rather than make a fresh invoker) - that is. deviceReporter.nameResolver.resolveName

[javihernandez]

@klown 🍻 🍻 🍻 🍻

[colinbdclark]

👏 woot @klown @javihernandez!

[klown]

c’est magnifique, @javihernandez. 🎶 (in honour of tuxguitar).