1025193 - Send a list of available commands to the server

Status: RESOLVED FIXED

(Firefox OS Graveyard :: FindMyDevice, defect)

Description

[Guilherme Gonçalves [:ggp]]

And report track as disabled when geolocation is disabled.

Comment 1

[JR Conlin [:jrconlin,:jconlin]]

During the "register" call, the client may send a list of device supported commands using the "accepts" keyword

The list of supported commands are:
erase -- device can be erased;
lock -- device is capable of locking;
ring -- device can ring or produce a sound on command;
track -- device can provide geographic tracking information;
has_lock -- device can report if it has a lock code set;

An example would be:
If a phone supported all functions except ring:
{"assert": ..., "pushurl": ..., "accepts":["erase", "lock", "track", "has_lock"]}

In addition, the list of command for "accepts" may be truncated to a single character.
e.g. for the above example:
{"assert": ..., "pushurl": ..., "accepts":["e","l","t","h"]}

Please note, that if a device attempts to later send a command that it has not specified it is capable of, the server will return an error.

Comment 2

[Guilherme Gonçalves [:ggp]]

The has_lock field will not be necessary in the "accepts" list, as the client already includes it in every response.

In the client, we should call /register with an updated "accepts" every time the list changes, which currently should only happen due to geolocation being enabled or disabled.

Comment 3

[JR Conlin [:jrconlin,:jconlin]]

That should be fine. Plus it lets us rotate the HAWK header for the client which should make opsec happy.

Comment 4

[Guilherme Gonçalves [:ggp]]

Attachment: gaia pull request

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to JR Conlin [:jrconlin,:jconlin] from comment #1)
> During the "register" call, the client may send a list of device supported
> commands using the "accepts" keyword
> 
> The list of supported commands are:
> erase -- device can be erased;
> lock -- device is capable of locking;
> ring -- device can ring or produce a sound on command;
> track -- device can provide geographic tracking information;
> has_lock -- device can report if it has a lock code set;
> 
> An example would be:
> If a phone supported all functions except ring:
> {"assert": ..., "pushurl": ..., "accepts":["erase", "lock", "track",
> "has_lock"]}
> 
> In addition, the list of command for "accepts" may be truncated to a single
> character.
> e.g. for the above example:
> {"assert": ..., "pushurl": ..., "accepts":["e","l","t","h"]}
> 
> Please note, that if a device attempts to later send a command that it has
> not specified it is capable of, the server will return an error.

Sending a character seems a lot of constraints in the future. Why can't we just sent integers that refers to specific commands ?

Comment 6

[Guilherme Gonçalves [:ggp]]

Oh look at that, I broke my own tests in the PR. That was sloppy. Updated it, let's see if it fares better this time.

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #5)
> Sending a character seems a lot of constraints in the future. Why can't we
> just sent integers that refers to specific commands ?

I think JR would be the best person to answer that, but note that we already use characters when the server tells the client which commands to run [1], so switching to numbers would be a pretty big change at this point, and I'd advise against doing it for 2.0.

In the long term, I don't particularly oppose it, but on the other hand I also don't really expect us to have so many commands that clashing initials will be a problem.

1- https://wiki.mozilla.org/Services/WheresMyFox#POST_.2F.3Cversion.3E.2Fcmd.2F.3Cdeviceid.3E

Comment 7

[Guilherme Gonçalves [:ggp]]

(In reply to Guilherme Gonçalves [:ggp] from comment #6)
> I'd advise against doing it for 2.0.

By the way, I had forgotten to mark this bug as blocking 1017268, which is nominated for blocking 2.0. That's why I mentioned 2.0 in my comment :)

Comment 8

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8441509 [details] [review]
gaia pull request

r+ but I would prefer the |reason| passed via IAC to be integer flags as well. You can have them as constants in the two files in order to have a better naming.

Comment 9

[Guilherme Gonçalves [:ggp]]

Will do, thanks!

Comment 10

[JR Conlin [:jrconlin,:jconlin]]

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #5)
> Sending a character seems a lot of constraints in the future. Why can't we
> just sent integers that refers to specific commands ?

Ideally, since this is not code meant for human readability, we'd use a binary protocol instead of something like JSON. There is, however, some consideration for making this new protocol's code somewhat readable because people are more comfortable with what they easily understand. In addition, I don't see the current implementation having more than a handful of commands available to it. Later versions may allow for complete remote executive power over the device, but we'll have a completely different protocol in place for that. 

To those ends, a character is a reasonable compromise.

Comment 11

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Travis is green, merging.

Master: https://github.com/mozilla-b2g/gaia/commit/470a8445bb4d82432b537662ddf4bfb8e968326a

Comment 12

[Preeti Raghunath(:Preeti)]

Erin

What is the user impact if this fix were not taken in 2.0?

Comment 13

[Erin Lancaster [:elan]]

This allows the device to honor when geolocation is disabled.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.0 uplift.

Comment 15

[Guilherme Gonçalves [:ggp]]

Attachment: v2.0 pull request

Comment 16

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/640fa4799913b0eb0016fbe149fc61c399c300c5