Bug 709125 - Review Request: obdgpslogger - OBDII and GPS data logger for your car
Summary: Review Request: obdgpslogger - OBDII and GPS data logger for your car
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: http://icculus.org/obdgpslogger/
Depends On:
TreeView+ depends on / blocked
Reported: 2011-05-30 18:00 UTC by Gary Briggs
Modified: 2012-01-08 07:40 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2012-01-08 07:40:04 UTC
Type: ---

Attachments (Terms of Use)

Description Gary Briggs 2011-05-30 18:00:19 UTC
Spec URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger.spec
SRPM URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger-0.16-1.fc15.src.rpm

obdgpslogger has been my pet project for a few years, now. It's a tool to log OBDII and GPS data from your car, and convert it into useful formats such as GPX or KML.

It's entirely scratching an itch; there's almost no software out there for working with OBDII as well as GPS, even less that's open source, and absolutely none that's made it into any distributions yet.

It also contains a simulator I've written, that's used more extensively than the logger itself; most of the OBDII software developers I know of, even the closed source ones, are now using my sim.

I've been building this on a Fedora 15 x86_64 VM. At time of writing, rpmlint(1) has no errors, and one warning: A spelling error, "obdsim", which I feel is excusable since it's the name of the sim. I see no errors or warnings from mock(1).

If you wish to test the software itself or see that it executes without problems, use obdsim with the "-o" flag. That will launch the sim [which creates a PTY on Linux], and connect obdgpslogger to the slave end of the PTY with verbose output turned on; if you see spam to stdout, that means that both the sim and the logger are functioning correctly. The warning, "close(9) on netlib_connectsock()", is created by the gpsd library, not my software.

Comment 1 Gary Briggs 2011-05-30 18:02:03 UTC
Adding FE-NEEDSPONSOR to blocking bugs, this will hopefully be my first package.

Comment 2 Volker Fröhlich 2011-06-03 23:24:07 UTC
Please remove the Requires. RPM is smart enough to figure out these two. Please put each BuildRequires on a separate line, as it is more legible.

If you don't plan to maintain your package in EPEL 5 or older, you can remove the buildroot definition and the buildroot cleaning. You can drop %defattr as well.

The files section could be a bit more specific -- Especially for the bindir.

Feel free to add the TODO file to %doc, if you find it useful.

make and rm macros are no longer used -- just use plain make and rm. Despite that, please use %cmake instead of cmake.

Please adapt your Cmake files to use the system's sqlite and also add it to the BRs. Therefore, also delete the tarball's libs directory in the prep section.

The package description should end in a full stop.

Comment 3 Volker Fröhlich 2011-06-03 23:33:33 UTC
Oh, didn't notice OBD_SQLITE_INCLUDED_LIB before.

Comment 4 Gary Briggs 2011-06-04 00:04:30 UTC
I would like to maintain the spec for as many distributions as possible. I dream of this making it into currently supported distribution versions as well as upcoming ones, so I'm inclined to leave the clean stuff there until distributions that require it are EOL'd.

The requires I left in there are ones that aren't libraries that get linked to; it's useful that users have the binaries that come in those packages, rather than just the library packages that rpm automatically figures out.

Is that OK, or will they just be inherited as "Suggests"-type packages from the libraries that *are* set as requirements?

[In case that was incoherent, an example: RPM will figure out that it requires libgpsd, but the gpsd daemon itself comes in a separate package that cannot be inferred from ldd]

I'll fix everything else tonight and push new versions of those files. Thank-you for the review

Comment 5 Gary Briggs 2011-06-04 04:52:44 UTC
OK, I have made all of the changes suggested. I have uploaded the files to the same location as before:

Spec URL:

Comment 6 Volker Fröhlich 2011-06-06 22:12:56 UTC
There is a spelling mistake in "Eearth". The last sentence of the description also seems a bit strange to me.

The make invocation should be:
make %{?_smp_mflags}

Please leave an explaining comment near where you delete the libs directory.

Whenever you publish an altered Spec file, please bump the release number. Don't upload a different file using the same release number, as it makes the reviewer's work harder. Besides that, you mustn't build two packages with the same release number afterwards, so it's good practice.

All relevant changes must also be noted in the changelog section of the spec file.

I didn't want to talk you out of maintaining your package in EPEL 5. Just wanted to let you know that some things are unnecessary if you don't want to. EPEL has a different policy on updating though: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Some_examples_of_what_package_updates_that_are_fine_or_not

RPM will install all packages that provide, what your package requires, as well as everything that is required therefore in turn. RPM does not have "suggests", as Debian has it. You can either require something or not.

You're absolutely right about these libraries not being linked. In the case of gpsd it works nevertheless:

$ rpm -qp --requires /home/makerpm/rpmbuild/RPMS/x86_64/obdgpslogger-0.16-1.fc14.x86_64.rpm

$ locate libgps.so.19

$ rpm -qf /usr/lib64/libgps.so.19

... which also provides libgpsd.

Comment 7 Gary Briggs 2011-06-07 04:45:45 UTC
I think I shall hold off on EPEL packaging; I feel there's more value in immediately targeting Fedora, and possibly worrying about backporting into EPEL later.

The new spec file is in the same place, the new SRPM is tagged with the new version suffix:

Spec URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger.spec
SRPM URL: http://icculus.org/obdgpslogger/downloads/fedorapackaging/obdgpslogger-0.16-2.fc15.src.rpm

Comment 8 Gary Briggs 2011-06-14 03:42:58 UTC
I asked in IRC about followups, they suggested another ping by appending a comment on this bug. So, erm, here it is.

Comment 9 Martin Gieseking 2011-07-21 13:30:45 UTC
Gary, the package is almost ready. Just add a .desktop file for "obdgui" as described here:

Have you already done some informal reviews? This is usually necessary to show that your familiar with the Fedora packaging guidelines. Since you're allowed to formally review and to approve other packager's submissions once you're sponsored into the packager group, you should know how to do that correctly. :)
I'm a potential sponsor, BTW.

Here are some more information about the sponsoring process and the guidelines:

In order to do a first informal review, please choose a yet uncommented review request not blocked by FE-NEEDSPONSOR, check all MUST and SHOULD items mentioned in the review guidelines (see link above), and post your results into the ticket.

Comment 10 Volker Fröhlich 2011-12-13 19:33:44 UTC
Any news, Gary?

Comment 11 Volker Fröhlich 2012-01-08 07:40:04 UTC
I'll close this ticket. If you re-gain interest, feel free to open it again.


Note You need to log in before you can comment on or make changes to this bug.