Closed Bug 1025193 Opened 8 years ago Closed 8 years ago

Send a list of available commands to the server

Categories

(Firefox OS Graveyard :: FindMyDevice, defect)

defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S4 (20june)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: ggp, Assigned: ggp)

References

Details

Attachments

(2 files)

46 bytes, text/x-github-pull-request
vingtetun
: review+
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
And report track as disabled when geolocation is disabled.
Blocks: 1002581
Target Milestone: --- → 2.0 S4 (20june)
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.
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.
That should be fine. Plus it lets us rotate the HAWK header for the client which should make opsec happy.
Attached file gaia pull request
Attachment #8441509 - Flags: review?(21)
(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 ?
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
Blocks: 1017268
(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 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.
Attachment #8441509 - Flags: review?(21) → review+
Will do, thanks!
Keywords: checkin-needed
(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.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
blocking-b2g: --- → 2.0?
Whiteboard: upliftneeded
Whiteboard: upliftneeded
Erin

What is the user impact if this fix were not taken in 2.0?
Flags: needinfo?(elancaster)
This allows the device to honor when geolocation is disabled.
Flags: needinfo?(elancaster)
blocking-b2g: 2.0? → 2.0+
Needs rebasing for v2.0 uplift.
Flags: needinfo?(ggoncalves)
Attached file v2.0 pull request
Flags: needinfo?(ggoncalves)
Depends on: 1034560
You need to log in before you can comment on or make changes to this bug.