Andreas said he is OK with applying them, so I will do that.
OK :)
However, if I read your code correctly, it still seems to me that there is a fixed compile time decision if gpsd or built-in gps support is to be used.
I think it would be better to keep it a runtime decision, i.e.
- if gpsd headers/library available during compilation, build support for _both_ gpsd and built-in gps into the program,
- if they are not available, only include the built-in gps support.
That's easy to do. I can maintain the original gps interface, so old code
doesn't need to be fixed, and choose the support to use from an internal state.
The decision which method to use should be a config file option. Please make sure that a config file configured for built-in gps support will work with both versions of the program.
A config file requesting the use of gpsd support should make the program abort if it was compiled without gpsd support included.
Not sure to agree. Execs from layer23/misc don't rely on config files.
Probably that's due to the low number of switches used. I think that, even if cell_log options are increased, we can still use cmd switches. Do you agree?
Ciao Dario.
Hi Dario,
On Thu, Feb 17, 2011 at 05:33:57PM +0100, Dario Lombardo wrote:
That's easy to do. I can maintain the original gps interface, so old code doesn't need to be fixed, and choose the support to use from an internal state.
Great.
A config file requesting the use of gpsd support should make the program abort if it was compiled without gpsd support included.
Not sure to agree. Execs from layer23/misc don't rely on config files. Probably that's due to the low number of switches used. I think that, even if cell_log options are increased, we can still use cmd switches. Do you agree?
Yes, I agree with you. No need for introducing a config file.
Dario Lombardo wrote:
+++ b/src/host/layer23/src/common/gps.c
..
@@ -343,11 +344,38 @@ void osmo_gps_close(void) gps_bfd.fd = -1; /* -1 or 0 indicates: 'close' */ }
-#endif
void osmo_gps_init(void) { memset(&gps_bfd, 0, sizeof(gps_bfd)); }
+int osmo_gps_open(void) +{
- switch (gps.gps_type) {
+#ifdef _HAVE_GPSD
case GPS_TYPE_GPSD:return osmo_gpsd_open();+#endif
case GPS_TYPE_SERIAL:return osmo_serialgps_open();default:return 0;- }
+}
Maybe the program should accept the gpsd options also when not compiled with support, and throw an error here? Benefit would be better consistency.
+++ b/src/host/layer23/src/misc/app_cell_log.c
..
static struct l23_app_info info = { .copyright = "Copyright (C) 2010 Andreas Eversberg\n", -#ifdef _USE_GPSD
- .getopt_string = "l:r:ng:p:",
+#ifdef _HAVE_GPSD
- .getopt_string = "l:r:nf:b:g:p:",
#else
- .getopt_string = "l:r:ng:b:",
- .getopt_string = "l:r:nf:b:",
#endif
This part *so* can not scale. Maybe use something like this..
.getopt_string = "l:r:nf:b" \ #ifdef _HAVE_GPSD "g:p:" \ #endif "",
+++ b/src/host/layer23/src/mobile/vty_interface.c @@ -855,7 +855,7 @@ DEFUN(cfg_no_gps_enable, cfg_no_gps_enable_cmd, "no gps enable", return CMD_SUCCESS; }
-#ifdef _USE_GPSD +#ifdef _HAVE_GPSD DEFUN(cfg_gps_host, cfg_gps_host_cmd, "gps host HOST:PORT",
..but again, maybe consider always having the options and commands be supported, and failing in some other way than invalid option/command without _HAVE_GPSD.
//Peter
Hi guys Thanks to Martin Hauke help I have added a gpsv3 support, that allows the previous support to work with gpsd-2.39 and gpsd-2.95.
To Peter: I have added your suggestion about the ".getopt_string", but not the suggestion about the runtime error, because I didn't find any smart way to do that. May be next step :).
Please take a look at the attached patch.
Bye Dario.
Last (hope definitive) patch for gpsd support that should include every suggestion. Feel free to comment. Dario.
Hi Andreas Please find attached the last version of the patch for gpsd support that incorporates the changes we said. Let me know if something needs to be changed. Bye Dario.
baseband-devel@lists.osmocom.org
-
Dario Lombardo -
Harald Welte -
Peter Stuge