Re: [gpsd-dev] [RFC] Add support for Airmar PB200 Weather station

[christian Gagneraud]

On 04/21/2012 04:23 PM, Eric S. Raymond wrote:
> address@hidden<address@hidden>:
> > The Airmar PB200 [1] is a weather station to be installed on boats.
> > It has built-in GPS, accelerometer, compass, gyro, anemometer, pressure and
> > temperature sensor. It has 2 outputs, one for NMEA0183 and one for
> > NMEA2000 (I've seen the recent addition for NMEA2000, I'm interesting
> > to make some tests and hacking around).
> >
> > It provides quite a lot of information and use the following NMEA sentences:
> >   - Usual GPS ones
> >   - TIROT, HCHDG and HCHDT for heading related data
> >   - WIMWD, WIMWV, WIVWR and WIVWT for wind related data
> >   - WIMDA for other meteorological data
> >   - WIXDR for detailed transducer measurements/calculations
> >
> > This is a WIP, I thought it might be worse to start the review early.
> > Things I didn't look at yet:
> >   - Update other tools (xgps, gpspipe, ....)
> >   - Update the testsuite(s)
> >   - Update documentation
> >
> > I naively added a wind data structure, but the WIMDA would require to
> > add yet another one (and in my case the WIXDR will bring even more
> > meteo data than the WIMDA). So maybe these two should be merged
> > together. Though, I didn't do it because there are several ways to
> > express the wind, both for the wind direction and for the wind speed
> > can be viewed from different referential frames: the moving boat, the
> > moving water or the fixed ground, and why not one of these days the
> > moving air mass (for flying things obviously).
> >
> > It is likely that I will come one day with yet another structure:
> > engine. I would like to install my openwrt/gpsd based system on a
> > boat, and it has two engines that speak NMEA200 only (I let you
> > imagine how high I jumped off my chair when I saw the new NMEA200 code
> > coming in... ;))
>
> I've merged the first two patches, and these patches in general look
> good - thorough, clean, and workmanlike.  I encourage you to continue.
> But as you note, there are some missing pieces.  Adding a device log
> to the regression-test suite is a requirement.  And yes, you do need
> to update the documentation.

OK, thanks. I will look at the documentation and the regression testsuite.

> There's a larger piece missing.  You've written JSON dumping for the
> daemon, but you also need to write the JSON parsing for the client
> side. That has to be in places before trying to modify cgps and xgps
> to display the new data will make any sense.

OK, I thought it was enough to change both gpsd_json.c (json_att_dump 
and json_wind_dump) and libgps_json.c (json_att_read and json_wind_read).
Could you maybe indicate me what I missed here?
I'll have a closer look anyway.

> I've just written a Hacking Guide section on how to add a sentence
> to the supported set.  It's a serious amount of work, and my hat is
> off to you for having deduced and done so many of the right things on
> your own.

Thanks, I've just read this new section. It's helpful.
I will have a look at the python binding and try to run splint/cppcheck 
and valgrind.

What is your feeling about adding more fields into the attitude 
structure? I will move them at the end of the structure, they were 
inserted there to keep a logical ordering.

And about the wind and meteo structures as well?
From your documentation you said it's not nice to break the ABI, which 
I fully agree and understand, but there no other ways around I can think of.

And last, do you have script to check source code against coding style, 
or even perhaps settings for emacs? I saw the reindent script in 
devtools, but the comments in there make me think it's not in use anymore.


Regards,
Chris