Add GPSD as a geolocation provider

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
Geolocation
--
enhancement
RESOLVED FIXED
8 years ago
a year ago

People

(Reporter: Martin McNickle, Assigned: dougt)

Tracking

({dev-doc-complete})

Trunk
mozilla1.9.3a1
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3
Build Identifier: N/A

GPSD[1] is a daemon for linux/unix that provides gps data from any gps devices attached to a machine. It makes this data available in a vendor independent form via TCP port 2947.

This would make an excellent geolocation provider, as it has the ability to provide velocity, altitiude and heading information as well as latitude and longitude.

[1]http://gpsd.berlios.de/

This should be included by default for linux/unix builds.


Reproducible: Always
(Reporter)

Comment 1

8 years ago
I've noticed that the Wifi provider was written in javascript, so maybe the GPSD provider doesn't need to be written in C after all.

I'm attacting my current js provider file as a base patch.

This patch needs robust error handling especially in regards to the socket code. It would also need some hooks to the preferences to set the GPSD IP address and port number.

Even still, could someone have a look at the code and see any potential problems with this approach?
(Reporter)

Comment 2

8 years ago
Created attachment 376684 [details]
GPSD Geolocation Provider
(Reporter)

Updated

8 years ago
Depends on: 454490
(Assignee)

Comment 3

8 years ago
Hi Martin, this looks good.  I want to fix 454490 so that multiple geolocation providers can exists at the same time.  I am pretty sure that we will break nsIGeolocationProvider when we do make this change.  I would love to have your input on this bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

8 years ago
Duplicate of this bug: 502857

Comment 5

8 years ago
Jo Hermans attached my (obvious duplicate) bug to this one.

Attached to #502857, there's another patch going a different route. It is implemented in C++ and calls an external program via popen(). That program is ment to return position information. In case there's a well-working JS provider, my patch might still be useful for debugging, because you can simply place a script that outputs whatever you want it to.

Comment 6

8 years ago
I'll attach that patch to this bug, too. It is not ment to exactly address gpsd-as-a-source problem, but to keep it in discussion as a generic provider for tests. (At least, I for one would love to see something like what my patch implements to be included. Would help me testing!)

Comment 7

8 years ago
Created attachment 387891 [details] [diff] [review]
Generic source reading location information from a popen()ed program.
(Assignee)

Comment 8

8 years ago
recently, i fixed 454490 that will allow many geolocation providers.

If you would like to create an addon and/or extension, AND if you would like GPSD to exist along side the default in Firefox, you can convert your registration to use a category instead of using the contract id.
(Reporter)

Comment 9

8 years ago
Created attachment 393793 [details] [diff] [review]
GPSD Geolocation Provider using Category Manager

I've modified the provider to add itself to the geolocation-provider category. I've tested and it does work in tandem with existing providers.

Have a look through and give any pointers to anything that needs some work.
Attachment #376684 - Attachment is obsolete: true
(Assignee)

Comment 10

8 years ago
Comment on attachment 393793 [details] [diff] [review]
GPSD Geolocation Provider using Category Manager

Martin, first awesome job.  really cool stuff.  I tried it out and it more or less worked great.  I had to tweak a few things, but otherwise great.  Here are the review notes I took looking over your code.  I am minusing this, but it is really close.

Let me know if something isn't clear.

remove references to the nsIGeolocationPrompt,  you do not need to implement that interface.

There is a dash in front of the Components.utils.import which will cause problems.

You want to comment out the dump's, or follow what I am doing in the NetworkGeolocationProvider wrt to logging.

startup() is called on the main thread which can block all UI.  is it safe to create a synchronous socket there?  Maybe it is better to do this asynchronously?  Afterall, here it doesn't look like startup() doesn't throw so you might as well just return and not write "J=1".  (not sure what j=1 does).  Maybe you can get away with just going into watcher mode right in startup().

You probably do not need to hold a reference to "this.stream", as the this.inputStream will own the "this.stream" reference.

Could you add comments about why you are writing what you are writing?  For example, what does writing "J=1\n" to the output stream mean?  I am sure that anyone with gpsd experience will know what is going on, but for most mozilla hackers this will look like voodoo.

Using the word "channel" as a variable name in networking code that isn't a nsIChannel is probably a bad idea.  Could you rename "var channel" to something else?  Maybe introString or something....

You could move the check for RMC above the looking over the fields as a minor optimization.

You probably should use the map() function to assign stuff into cords similar to what I do in the other provider.

style nits.  use spaces not tabs.  two spaces works great.  similar to what the NetworkGelocationProvider does.
Attachment #393793 - Flags: review-
(Reporter)

Comment 11

8 years ago
Created attachment 394817 [details] [diff] [review]
GPSD Geolocation Provider
Attachment #393793 - Attachment is obsolete: true
Attachment #394817 - Flags: review?(doug.turner)
(Reporter)

Comment 12

8 years ago
(In reply to comment #10)
> remove references to the nsIGeolocationPrompt,  you do not need to implement
> that interface.
> 
> There is a dash in front of the Components.utils.import which will cause
> problems.
> 
> You want to comment out the dump's, or follow what I am doing in the
> NetworkGeolocationProvider wrt to logging.

Done.

> startup() is called on the main thread which can block all UI.  is it safe to
> create a synchronous socket there?  Maybe it is better to do this
> asynchronously?  Afterall, here it doesn't look like startup() doesn't throw so
> you might as well just return and not write "J=1".  (not sure what j=1 does). 
> Maybe you can get away with just going into watcher mode right in startup().

The streams that are opened there are non-blocking so it should be safe to open there.

I've moved the "J=1" (set gpsd buffer on) to watch() now as well.

> You probably do not need to hold a reference to "this.stream", as the
> this.inputStream will own the "this.stream" reference.

I was getting mixed up here. Now holds a reference to inputStream and outputStream only. I create a scriptableInputStream inside watch() now.

> Could you add comments about why you are writing what you are writing?  For
> example, what does writing "J=1\n" to the output stream mean?  I am sure that
> anyone with gpsd experience will know what is going on, but for most mozilla
> hackers this will look like voodoo.
> 
> Using the word "channel" as a variable name in networking code that isn't a
> nsIChannel is probably a bad idea.  Could you rename "var channel" to something
> else?  Maybe introString or something....
> 
> You could move the check for RMC above the looking over the fields as a minor
> optimization.

Done.

> You probably should use the map() function to assign stuff into cords similar
> to what I do in the other provider.

I think I've addressed this, but let me know if I've misunderstood.

> style nits.  use spaces not tabs.  two spaces works great.  similar to what the
> NetworkGelocationProvider does.

Should've caught this myself. Fixed.

This should probably get minused because:

a) I need help with adding the "include for LINUX/UNIX only" directives, and the patch should be included without it.
b) Not sure if I should go cluttering up the preferences with geo.gpsd.*

I'd appreciate a pointer on these last 2 things.

Comment 13

8 years ago
Is a GPSD geolocation provider really *only* good for Linux? You'd connect from a Windows box to a GPSD running on a different machine. The other Unix derivates might want it, too. And I seem to remember that there's a GPSDalike running natively on windows with a compatible protocol.
(Reporter)

Comment 14

8 years ago
@Jan

I didn't consider that case; but you are correct. There is nothing Linux/Unix specific in the code so it should be included for all OSes.

Thanks for pointing that out.
(Reporter)

Updated

8 years ago
OS: Linux → All
Hardware: x86 → All
(Assignee)

Comment 15

8 years ago
Comment on attachment 394817 [details] [diff] [review]
GPSD Geolocation Provider

lets make default prefs so that you do not have to add anything to the preferences file.  That is:


+    var hostIPAddr = this.prefService.getCharPref("geo.gpsd.host.ipaddr");

becomes:

var hostIPAddr = "localhost";

try {
  hostIPAddr  = this.prefService.getCharPref("geo.gpsd.host.ipaddr");
}
catch (e) {}


Otherwise, i think this is fine.  Throw me one more patch.
Attachment #394817 - Flags: review?(doug.turner) → review-
(Reporter)

Comment 16

8 years ago
Created attachment 395325 [details] [diff] [review]
GPSD Geolocation Provider

Avoids cluttering preferences by assuming default values for ipaddr and port.
Attachment #394817 - Attachment is obsolete: true
Attachment #395325 - Flags: review?(doug.turner)
(Assignee)

Updated

8 years ago
Attachment #395325 - Flags: review?(doug.turner) → review+
(Assignee)

Comment 17

8 years ago
http://hg.mozilla.org/mozilla-central/rev/44344ac8fe41

martin, can you work with Joel to help him create a litmus test?
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 18

8 years ago
Comment on attachment 395325 [details] [diff] [review]
GPSD Geolocation Provider

safe support of GPSD.  we should consider for FF3.6.
Attachment #395325 - Flags: approval1.9.2?
Assignee: nobody → mmcnicklebugs
You didn't add GPSDGeolocationProvider.js to any of the packaging manifests, so this file is only shipping in our Mac builds, currently:
http://mxr.mozilla.org/mozilla-central/source/browser/installer/windows/packages-static
http://mxr.mozilla.org/mozilla-central/source/browser/installer/unix/packages-static
Attachment #395325 - Flags: approval1.9.2? → approval1.9.2-
Comment on attachment 395325 [details] [diff] [review]
GPSD Geolocation Provider

a- until you fix the packaging manifests.
(Assignee)

Comment 21

8 years ago
Created attachment 395913 [details] [diff] [review]
packaging fu.
Assignee: mmcnicklebugs → doug.turner
Attachment #395913 - Flags: review?
(Assignee)

Updated

8 years ago
Attachment #395913 - Flags: review? → review?(ted.mielczarek)
Attachment #395913 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 22

8 years ago
packaging fu:

http://hg.mozilla.org/mozilla-central/rev/c0e002e1d5e9
(Assignee)

Updated

8 years ago
Attachment #395325 - Flags: approval1.9.2- → approval1.9.2?
(Assignee)

Comment 23

8 years ago
Comment on attachment 395325 [details] [diff] [review]
GPSD Geolocation Provider

rerequesting now that packaging is correct.

Updated

8 years ago
Blocks: 512005
(Reporter)

Comment 24

8 years ago
> martin, can you work with Joel to help him create a litmus test?

Sure. Although I'm not sure how to create a test to test individual providers as they are all exposed through the same interface. Any ideas?

Joel, if you need any questions answered please contact me directly.
Martin, I can easily create a case (or set of cases) that will cover this.  I need to know how to access a GPSD provider?  Is there a specific website that will do this?  

Do we need to install software on a local computer to interact with GPSD?  If you can give me a set of steps that will allow me to test this, I can get it added to litmus so we run it for each release.
(Reporter)

Comment 26

8 years ago
Joel,

There is software to install in order to test the provider. However this doesn't have to be installed locally. You could set up a remote computer to run GPSD and have the test machine connect to it:

Say that the remote machine has an IP address of 123.123.123.123 then the test would look like the following.

1) Navigate to 'about:config'
2) Create a new string preference, name 'geo.gpsd.host.ipaddr' value '123.123.123.123'
3) Create a new boolean value, name 'geo.gpsd.logging.enabled' value 'true'
4) Open the error console, select 'messages'
5) Navigate to 'http://people.mozilla.org/~dougt/geo.html'
6) Accept request to acquire position
7) Verify that there are messages in the error console starting with '*** GPSD GEO: Got info: ...'

This test will ensure that the provider is working and reporting position.

The different geoproviders compete to report position; the one that has the "best" position wins. There may be cases where the Network provider will "win" and as a result the position reported by the site will NOT have come from the GPSD provider. It should be enough to ensure that it is generating the '*** GPSD GEO: Got info: ...' messages, it DOES NOT need to match the coordindates displayed on the site.

Setting up GPSD to run on a linux box is trivial, and you can get it to report fake data which is good for testing purposes. The problem may be finding a machine to host it reliably. Any ideas?

Updated

8 years ago
Attachment #395325 - Flags: approval1.9.2? → approval1.9.2+
(Assignee)

Comment 27

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6d700cea1785
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/39cd8bd62eaf
Keywords: fixed1.9.2
status1.9.2: --- → beta1-fixed
Keywords: fixed1.9.2
Target Milestone: --- → mozilla1.9.3a1
Version: unspecified → Trunk

Comment 28

8 years ago
As of rev:39cd8bd62eaf it seems to be parsing the RMC NMEA sentence, but the field names in the code don't match the NMEA specification for RMC - for example there's no horizontal and vertical error. 
To calculate the error you'd probably have to make a guess from the PDOP value. Or you'd want to use GPSD over wifi positioning as long as the GPS has a valid fix?
(Reporter)

Comment 29

8 years ago
(In reply to comment #28)
> As of rev:39cd8bd62eaf it seems to be parsing the RMC NMEA sentence, but the
> field names in the code don't match the NMEA specification for RMC.

The code isn't really parsing the RMC sentence. GPSD provides a layer of abstraction from the NMEA sentences, so what is being parsed is actually just a GPSD output. With caching on, GPSD will combine various sentences to provide a complete time/position/velocity report as one output[1]. We just happen to update the position when an new RMC sentence is recieved.

[1] http://gpsd.berlios.de/gpsd.html see option 'o' under the 'OLD PROTOCOL' section.

Comment 30

8 years ago
(In reply to comment #29)

> The code isn't really parsing the RMC sentence. GPSD provides a layer of
> abstraction from the NMEA sentences, so what is being parsed is actually just a
> GPSD output. With caching on, GPSD will combine various sentences to provide a
> complete time/position/velocity report as one output[1]. We just happen to
> update the position when an new RMC sentence is recieved.

Ah, I see - I was misinterpreting line 239. Since GPSD is only reporting the last NMEA setenced parsed, is it even needed to check for the RMC sentence, as all GPSD "O" messages should have the same contents?

I ask because my GPSD server does not output RMC, only GGA, GSA, and GSV.

I figure that you select one sentence instead of parsing the same "O" output multiple times per GPS position update? Maybe the code should grab the first valid NMEA tag it sees and then just keep using that one?

This is very nice work and exactly what I need for a project right now. If any assistance is needed I have time available to devote to this.
(Reporter)

Comment 31

8 years ago
Hi Marc, thanks for the kind comments.

I have only the one device that I can test with, so I was not aware of this problem. Would it be possible for you to open a bug about it? It would also be really useful if you uploaded an NMEA dump along with it for testing.

Comment 32

8 years ago
Martin, I opened a bug with sample data:

https://bugzilla.mozilla.org/show_bug.cgi?id=524722

I can also make our GPSD test server public if needed for testing.

Delving further into this it appears for some reason GPSD is not outputting the RMC tag in my test data in 'watcher' mode.
Keywords: dev-doc-needed
Added a note mentioning this support here:

https://developer.mozilla.org/En/Using_geolocation
Keywords: dev-doc-needed → dev-doc-complete

Comment 34

8 years ago
Any ideas why the support for gpsd as a geolocation provider is only on Linux.
There are already ports of gpsd on Windows and I feel that the support for gpsd should be turned on by default for Windows also.

GPSd support on Windows
http://home.arcor.de/ulf.lamping/gpsd/gpsd.html
http://www.renderlab.net/projects/wrt54g/gpsdonwindows.html
See Also: → bug 1250922
You need to log in before you can comment on or make changes to this bug.