RE: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
Hi Waldo,
On Fri, 2010-12-24 at 10:54 -0800, ext Bastian, Waldo wrote:
+struct gsm {
+ int lac;
+ int ci;
+ int ta;
+ int no_cells;
+ struct geran_neigh_cell{
+ int arfcn;
+ int bsic;
+ int rxlev;
+
+ } nmr[OFONO_MAX_NMR_COUNT];
+};
What about the frequency of the serving cell? I was under the impression that
there was a need for that one as well.
Even though the serving cell may be included already in the nmr, do you see a
need
to explicitly provide the arfcn, bsic and rxlev of the serving cell as well?
Serving cell frequency is, or should be, included in the nmr list in the
arfcn variable. I believe the arfcn, bsic and rxlev are needed for all
cells since this information is used for location determination.
What about LAC and CI for the neighbouring cells?
As far as I know, the neighbor cell info in the above-mentioned form is
used for location specific purposes. Hence, bsic suffices to identify
base station and rxlev gives the unscaled distance from it.
If there is a reason for including LAC and CI for neighboring cells, we
can add them to the list. In that case we should probably modify the
DBUS method list to include one method for requesting location data and
another method for general identity data.
Best Regards,
Antti
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
+struct gsm {
+ int lac;
+ int ci;
+ int ta;
+ int no_cells;
+ struct geran_neigh_cell{
+ int arfcn;
+ int bsic;
+ int rxlev;
+
+ } nmr[OFONO_MAX_NMR_COUNT];
+};
What about the frequency of the serving cell? I was under the impression that
there was a need for that one as well.
Even though the serving cell may be included already in the nmr, do you see a
need
to explicitly provide the arfcn, bsic and rxlev of the serving cell as well?
What about LAC and CI for the neighbouring cells?
Cheers,
Waldo
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
Hi Antti,
---
include/cell-info.h | 133
+++
1 files changed, 133 insertions(+), 0 deletions(-) create mode
100644 include/cell-info.h
diff --git a/include/cell-info.h b/include/cell-info.h new file mode
100644 index 000..3941e69
--- /dev/null
+++ b/include/cell-info.h
@@ -0,0 +1,133 @@
+/*
...
...
...
+struct ofono_cell_info_driver {
+ const char *name;
+ int (*probe)(struct ofono_cell_info *ci,
+ unsigned int vendor,
+ void *data);
+ void (*remove)(struct ofono_cell_info *ci);
+ void (*query)(struct ofono_cell_info *ci,
+ ofono_cell_info_query_cb_t,
+ void *data);
+};
+
To be aligning with the other header files, do we need to add one 'cb'
after 'ofono_cell_info_query_cb_t' here?
This seems to be a matter with no clear convention. In sms.h and
netreg.h some callbacks have 'cb' and some not. I agree that if within
a function's parameter list some parameters have name declared, so
should all have.
we are trying to be consistent, but some of these slipped through in the
initial development phase of oFono.
Once we find them, we document them in doc/coding-style.txt and then
slowly start fixing these up. This is an ongoing process and pretty hard
if your codebase reaches over 100k lines of code.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
Hi Antti,
On Wed, 2010-12-22 at 11:35 +0200, ext Antti Paila wrote:
---
include/cell-info.h | 133
+++
1 files changed, 133 insertions(+), 0 deletions(-)
create mode 100644 include/cell-info.h
diff --git a/include/cell-info.h b/include/cell-info.h
new file mode 100644
index 000..3941e69
--- /dev/null
+++ b/include/cell-info.h
@@ -0,0 +1,133 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2010 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA
+ *
+ */
+
+#ifndef __OFONO_CELL_INFO_H
+#define __OFONO_CELL_INFO_H
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+#include ofono/types.h
+
+struct ofono_cell_info;
+
+#define OFONO_CI_FIELD_UNDEFINED -1001
+#define OFONO_MAX_NMR_COUNT 15
+#define OFONO_MAX_MEASURED_CELL_COUNT 32
+#define OFONO_MAX_MEAS_RES_LIST_COUNT 8
+
+
+enum ofono_cell_type {
+ OFONO_CELL_TYPE_GERAN,
+ OFONO_CELL_TYPE_UTRA_FDD
+};
+
+enum UTRA_MEAS_TYPE {
+ UTRA_MEAS_TYPE_INTER_FREQ,
+ UTRA_MEAS_TYPE_INTRA_FREQ,
+};
+
+struct gsm {
+ int lac;
+ int ci;
+ int ta;
+ int no_cells;
+ struct geran_neigh_cell{
+ int arfcn;
+ int bsic;
+ int rxlev;
+
+ } nmr[OFONO_MAX_NMR_COUNT];
+};
+
+struct cell_measured_results {
+ int ucid;
+ int sc;
+ int ecn0;
+ int rscp;
+ int pathloss;
+};
+
+struct measured_results_list {
+ int dl_freq;
+ int ul_freq;
+ int rssi;
+ int no_cells;
+ struct cell_measured_results cmr[OFONO_MAX_MEASURED_CELL_COUNT];
+};
+
+struct wcdma {
+ int ucid;
+ int dl_freq;
+ int ul_freq;
+ int sc;
+ int no_freq;
+ struct measured_results_list mrl[OFONO_MAX_MEAS_RES_LIST_COUNT];
+
+};
Since these things are called GERAN and UTRAN over D-Bus, it would make
sense to use the same naming here instead of struct gsm and struct
wcdma.
+struct ofono_cell_info_results {
+ enum ofono_cell_type rat;
+ int mcc;
+ int mnc;
+ union {
+ struct gsm gsm;
+ struct wcdma wcdma;
+ };
+
+};
+
+
+typedef void (*ofono_cell_info_query_cb_t)(const struct ofono_error *error,
+ struct ofono_cell_info_results results, void *data);
The results is a pointer.
+struct ofono_cell_info_driver {
+ const char *name;
+ int (*probe)(struct ofono_cell_info *ci,
+ unsigned int vendor,
+ void *data);
+ void (*remove)(struct ofono_cell_info *ci);
+ void (*query)(struct ofono_cell_info *ci,
+ ofono_cell_info_query_cb_t,
+ void *data);
+};
+
+struct ofono_cell_info *ofono_cell_info_create(struct ofono_modem *modem,
+ unsigned int vendor,
+ const char *driver,
+ void *data);
+
+void ofono_cell_info_register(struct ofono_cell_info *ci);
+void ofono_cell_info_remove(struct ofono_cell_info *ci);
+int ofono_cell_info_driver_register(struct ofono_cell_info_driver *driver);
+void ofono_cell_info_driver_unregister(struct ofono_cell_info_driver
*driver);
+void *ofono_cell_info_get_data(struct ofono_cell_info *ci);
+void ofono_cell_info_set_data(struct ofono_cell_info *ci, void *cid);
+void ofono_cell_info_query_cb(const struct ofono_error *error,
+ struct ofono_cell_info_results results, void *data);
Why is this last one public?
Cheers,
Aki
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
On Wed, 2010-12-22 at 20:45 +0200, ext Aki Niemi wrote:
Hi Antti,
On Wed, 2010-12-22 at 11:35 +0200, ext Antti Paila wrote:
---
include/cell-info.h | 133
+++
1 files changed, 133 insertions(+), 0 deletions(-)
create mode 100644 include/cell-info.h
diff --git a/include/cell-info.h b/include/cell-info.h
new file mode 100644
index 000..3941e69
--- /dev/null
+++ b/include/cell-info.h
@@ -0,0 +1,133 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2010 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA
+ *
+ */
+
+#ifndef __OFONO_CELL_INFO_H
+#define __OFONO_CELL_INFO_H
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+#include ofono/types.h
+
+struct ofono_cell_info;
+
+#define OFONO_CI_FIELD_UNDEFINED -1001
+#define OFONO_MAX_NMR_COUNT 15
+#define OFONO_MAX_MEASURED_CELL_COUNT 32
+#define OFONO_MAX_MEAS_RES_LIST_COUNT 8
+
+
+enum ofono_cell_type {
+ OFONO_CELL_TYPE_GERAN,
+ OFONO_CELL_TYPE_UTRA_FDD
+};
+
+enum UTRA_MEAS_TYPE {
+ UTRA_MEAS_TYPE_INTER_FREQ,
+ UTRA_MEAS_TYPE_INTRA_FREQ,
+};
+
+struct gsm {
+ int lac;
+ int ci;
+ int ta;
+ int no_cells;
+ struct geran_neigh_cell{
+ int arfcn;
+ int bsic;
+ int rxlev;
+
+ } nmr[OFONO_MAX_NMR_COUNT];
+};
+
+struct cell_measured_results {
+ int ucid;
+ int sc;
+ int ecn0;
+ int rscp;
+ int pathloss;
+};
+
+struct measured_results_list {
+ int dl_freq;
+ int ul_freq;
+ int rssi;
+ int no_cells;
+ struct cell_measured_results cmr[OFONO_MAX_MEASURED_CELL_COUNT];
+};
+
+struct wcdma {
+ int ucid;
+ int dl_freq;
+ int ul_freq;
+ int sc;
+ int no_freq;
+ struct measured_results_list mrl[OFONO_MAX_MEAS_RES_LIST_COUNT];
+
+};
Since these things are called GERAN and UTRAN over D-Bus, it would make
sense to use the same naming here instead of struct gsm and struct
wcdma.
+struct ofono_cell_info_results {
+ enum ofono_cell_type rat;
+ int mcc;
+ int mnc;
+ union {
+ struct gsm gsm;
+ struct wcdma wcdma;
+ };
+
+};
+
+
+typedef void (*ofono_cell_info_query_cb_t)(const struct ofono_error *error,
+ struct ofono_cell_info_results results, void *data);
The results is a pointer.
Should be.
+struct ofono_cell_info_driver {
+ const char *name;
+ int (*probe)(struct ofono_cell_info *ci,
+ unsigned int vendor,
+ void *data);
+ void (*remove)(struct ofono_cell_info *ci);
+ void (*query)(struct ofono_cell_info *ci,
+ ofono_cell_info_query_cb_t,
+ void *data);
+};
+
+struct ofono_cell_info *ofono_cell_info_create(struct ofono_modem *modem,
+ unsigned int vendor,
+ const char *driver,
+ void *data);
+
+void ofono_cell_info_register(struct ofono_cell_info *ci);
+void ofono_cell_info_remove(struct ofono_cell_info *ci);
+int ofono_cell_info_driver_register(struct ofono_cell_info_driver *driver);
+void ofono_cell_info_driver_unregister(struct ofono_cell_info_driver
*driver);
+void *ofono_cell_info_get_data(struct ofono_cell_info *ci);
+void ofono_cell_info_set_data(struct ofono_cell_info *ci, void *cid);
+void ofono_cell_info_query_cb(const struct ofono_error *error,
+ struct ofono_cell_info_results results, void *data);
Why is this last one public?
You are right. It doesn't need to be public. Thanks for the comments.
Best Regards,
Antti
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
Hi Antti,
On Wed, 2010-12-22 at 11:35 +0200, ext Antti Paila wrote:
---
include/cell-info.h | 133
+++
1 files changed, 133 insertions(+), 0 deletions(-) create mode
100644 include/cell-info.h
diff --git a/include/cell-info.h b/include/cell-info.h new file mode
100644 index 000..3941e69
--- /dev/null
+++ b/include/cell-info.h
@@ -0,0 +1,133 @@
+/*
...
...
...
+struct ofono_cell_info_driver {
+ const char *name;
+ int (*probe)(struct ofono_cell_info *ci,
+ unsigned int vendor,
+ void *data);
+ void (*remove)(struct ofono_cell_info *ci);
+ void (*query)(struct ofono_cell_info *ci,
+ ofono_cell_info_query_cb_t,
+ void *data);
+};
+
To be aligning with the other header files, do we need to add one 'cb' after
'ofono_cell_info_query_cb_t' here?
Regards,
Wei,Jun
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [RFC PATCH 2/5 v2] cell-info: Header file for cell-info atom
Hi Jun,
On Thu, 2010-12-23 at 14:31 +0800, ext Wei, Jun wrote:
Hi Antti,
On Wed, 2010-12-22 at 11:35 +0200, ext Antti Paila wrote:
---
include/cell-info.h | 133
+++
1 files changed, 133 insertions(+), 0 deletions(-) create mode
100644 include/cell-info.h
diff --git a/include/cell-info.h b/include/cell-info.h new file mode
100644 index 000..3941e69
--- /dev/null
+++ b/include/cell-info.h
@@ -0,0 +1,133 @@
+/*
...
...
...
+struct ofono_cell_info_driver {
+ const char *name;
+ int (*probe)(struct ofono_cell_info *ci,
+ unsigned int vendor,
+ void *data);
+ void (*remove)(struct ofono_cell_info *ci);
+ void (*query)(struct ofono_cell_info *ci,
+ ofono_cell_info_query_cb_t,
+ void *data);
+};
+
To be aligning with the other header files, do we need to add one 'cb' after
'ofono_cell_info_query_cb_t' here?
This seems to be a matter with no clear convention. In sms.h and
netreg.h some callbacks have 'cb' and some not. I agree that if within
a function's parameter list some parameters have name declared, so
should all have.
Thanks for the comment.
Best Regards,
Antti
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
