Re: Write state of SMS assembly to disk and restore on startup.
Hi Andrew,
On Fri, Aug 21, 2009 at 9:40 PM, Andrzej Zaborowski
andrew.zaborow...@intel.com wrote:
This way we can continue receiving segmented messages over a reset or
crash.
---
src/common.c | 29 +++
src/common.h | 10 +++
src/sim.c| 34
src/sms.c| 14 +++-
src/smsutil.c| 237
+-
src/smsutil.h|3 +-
unit/Makefile.am |6 +-
unit/test-sms.c |4 +-
8 files changed, 293 insertions(+), 44 deletions(-)
Somehow this patch fell through the cracks completely. Can you rebase and
resend?
+
+int create_dirs(const char *filename, const mode_t mode)
+{
Can we move this part to storage.c instead of common.c to be more consistent
with connman bluez?
+
+#ifdef TEMP_FAILURE_RETRY
+#define TFR TEMP_FAILURE_RETRY
+#else
+#define TFR
+#endif
+
+#include fcntl.h
+
+int create_dirs(const char *filename, const mode_t mode);
storage.c as well.
+static GSList *sms_assembly_add_fragment_backup(struct sms_assembly
*assembly,
+ const struct sms *sms, time_t ts,
+ const struct sms_address *addr,
+ guint16 ref, guint8 max, guint8
seq,
+ gboolean backup);
+
+#define SMS_BACKUP_MODE 0600
+#define SMS_BACKUP_PATH STORAGEDIR /%s/sms
+#define SMS_BACKUP_PATH_DIR SMS_BACKUP_PATH /%s-%i-%i
+#define SMS_BACKUP_PATH_FILE SMS_BACKUP_PATH_DIR /%03i
+
+#define SMS_ADDR_FMT %21[0-9+*#]
These really belong at the top of the file. Includes, Defines, globals,
forward declarations, structure definitions in that order.
+
+static void sms_assembly_load(struct sms_assembly *assembly,
+ const struct dirent *dir)
+{
+ for (; len--; free(segments[len])) {
Please don't do such evilness. I don't even understand how this can work.
+ fd = TFR(open(path, O_RDONLY));
+ g_free(path);
+
+ if (fd == -1)
+ continue;
+
+ if (fstat(fd, segment_stat) != 0) {
+ TFR(close(fd));
+ continue;
+ }
+
+ r = TFR(read(fd, buf, sizeof(buf)));
This part should be a utility function in storage.c. We repeat almost the
same code in sim.c
+
+ if (r 0 sms_deserialize(buf, segment, r)) {
+ if (sms_assembly_add_fragment_backup(assembly,
+ segment,
+ segment_stat.st_mtime,
+ addr, ref, max, seq,
FALSE)) {
+ /* This can't happen */
+ }
We read it from the backup store and almost immediately overwrite the backup
store? Seems inefficient.
+ if (create_dirs(path, SMS_BACKUP_MODE | S_IXUSR)) {
+ g_free(path);
+ return FALSE;
+ }
+
+ fd = TFR(open(path, O_WRONLY | O_CREAT, SMS_BACKUP_MODE));
+ if (fd == -1) {
+ g_free(path);
+ return FALSE;
+ }
+
+ if (TFR(write(fd, buf, len)) len) {
+ TFR(close(fd));
+ unlink(path);
+ g_free(path);
+ return FALSE;
+ }
+
+ g_free(path);
+ TFR(close(fd));
Again, sounds like this should be a utility function in storage.c
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] G1: Add initial HTC G1 modem support
Hi Andres,
This series adds support for the HTC G1 phone (that is, the Google
phone).
G1 plugin is based on generic_at, with a bunch of stuff dropped
and simplified. We use AT+CFUN=1 for powering on rather than having
a configurable init string. We also manually set the default state
during init (the G1 appears to start in mode V0 by default). The
device (/dev/smd0) is hardcoded.
+static gboolean connect_cb(GIOChannel *io, GIOCondition cond, gpointer
user) +{
+ struct ofono_modem *modem = user;
+ struct g1_data *d = ofono_modem_get_data(modem);
+ int err = 0;
+ gboolean success;
+ GAtSyntax *syntax;
+
+ if (cond G_IO_NVAL)
+ return FALSE;
+
+ if (cond G_IO_OUT) {
+ int sock = g_io_channel_unix_get_fd(io);
+ socklen_t len = sizeof(err);
+
+ if (getsockopt(sock, SOL_SOCKET, SO_ERROR, err, len) 0)
+ err = errno == ENOTSOCK ? 0 : errno;
+ } else if (cond (G_IO_HUP | G_IO_ERR))
+ err = ECONNRESET;
+
+ success = !err;
+
+ DBG(io ref: %d, io-ref_count);
+
+ if (success == FALSE)
+ goto error;
+
+ syntax = g_at_syntax_new_gsmv1();
+ d-chat = g_at_chat_new(io, syntax);
+ g_at_syntax_unref(syntax);
+
+ DBG(io ref: %d, io-ref_count);
+
+ if (!d-chat)
+ goto error;
+
Since this is a tty, you probably don't need this function at all.
+static GIOChannel *tty_connect(const char *tty)
+{
+ GIOChannel *io;
+ int sk;
+ struct termios newtio;
+
+ sk = open(tty, O_RDWR | O_NOCTTY);
+
+ if (sk 0) {
+ ofono_error(Can't open TTY %s: %s(%d),
+ tty, strerror(errno), errno);
+ return NULL;
+ }
+
+ newtio.c_cflag = B115200 | CRTSCTS | CLOCAL | CREAD;
+ newtio.c_iflag = IGNPAR;
+ newtio.c_oflag = 0;
+ newtio.c_lflag = 0;
+
+ newtio.c_cc[VTIME] = 1;
+ newtio.c_cc[VMIN] = 5;
+
+ tcflush(sk, TCIFLUSH);
+ if (tcsetattr(sk, TCSANOW, newtio) 0) {
+ ofono_error(Can't change serial settings: %s(%d),
+ strerror(errno), errno);
+ close(sk);
+ return NULL;
+ }
+
+ io = g_io_channel_unix_new(sk);
+ g_io_channel_set_close_on_unref(io, TRUE);
+
+ if (g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK,
+ NULL) != G_IO_STATUS_NORMAL) {
+ g_io_channel_unref(io);
+ return NULL;
+ }
+
+ return io;
+}
You might want to see if using g_at_chat_new_from_tty works here. If not,
then the opening the tty should be broken out into a separate utility function
that can be used from multiple drivers.
+static int g1_enable(struct ofono_modem *modem)
+{
+ struct g1_data *d = ofono_modem_get_data(modem);
+ GIOChannel *io;
+ GIOCondition cond;
+
+ DBG();
+
+ io = tty_connect(MODEM_DEVICE);
+ if (io == NULL)
+ return -EINVAL;
+
+ DBG(io ref: %d, io-ref_count);
+
+ cond = G_IO_OUT | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
+ g_io_add_watch_full(io, G_PRIORITY_DEFAULT, cond, connect_cb,
+ modem, connect_destroy);
+
+ DBG(io ref: %d, io-ref_count);
+
+ g_io_channel_unref(io);
+
+ DBG(io ref: %d, io-ref_count);
+ d-io = io;
+
+ return -EINPROGRESS;
+}
So I know the generic_at driver does this, but in a real device driver this is
wrong. You should probably be using AT+CFUN=1 here. If the g1 supports even
more drastic power up/down options, then these should be used.
+
+static int g1_disable(struct ofono_modem *modem)
+{
+ struct g1_data *d = ofono_modem_get_data(modem);
+
+ if (d-io) {
+ g_io_channel_close(d-io);
+ d-io = NULL;
+ }
+
+ if (d-chat) {
+ g_at_chat_unref(d-chat);
+ d-chat = NULL;
+ }
+
+ return 0;
+}
Same thing here. AT+CFUN=0 or AT+CFUN=4.
+static int g1_init(void)
+{
+ int err;
+
+ err = ofono_modem_driver_register(g1_driver);
+ if (err)
+ goto done;
+
+ g1_modem = ofono_modem_create(G1, HTC G1);
+ if (!g1_modem) {
+ err = -EIO;
+ goto unreg;
+ }
+
+ err = ofono_modem_register(g1_modem);
+ if (err)
+ goto remove;
+
+ return 0;
+
+remove:
+ ofono_modem_remove(g1_modem);
+unreg:
+ ofono_modem_driver_unregister(g1_driver);
+done:
+ return err;
+}
+
I'd really like to see some sort of detection mechanism here. Even if it is a
basic 'if exists /dev/smd0' or reading something from /proc. As I don't
necessarily want to see the g1 showing up everywhere.
Regads,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/3] G1: Add a G1 syntax for parsing
Hi Andres, This is based on the generic_at parser, with unnecessary stuff removed. The G1 routinely screws up CRLFs, so the parser needs to account for that. This parser ignores leading CRLFs (which is what reference-ril does as well), as well as trailing LFs (which are sometimes left out). CRs are used as end-of-message indicators. Since we're not bothering tracking CRLFs, there's also no need for a GARBAGE state, or MULTILINE stuff. --- This one looks fine to me. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/3] G1: Add G1-specific SMS support
Hi Andres, This is based on the atmodem SMS code, with lots of stuff dropped. The G1's modem advertises support for CNMI mode 2, but attempting to set that actually fails. Instead, we _need_ to use mode 1. Along those lines, CNMA doesn't appear to work properly, so we disable it and use CMTI rather than CMT for PDUs. CDS notifications (for status notifications) are disabled completely. So as we discussed on IRC, this one should really be rolled up into the generic_at sms driver, since the only change of substance is the cnmi mode selection specific to the g1. We need a driver quirk system to be figured out first though. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Ismo Puustinen a écrit : I started working on Ofono GPRS support. Hello, With GPRS support, Ofono become very interesting! I failed to understand how the isi_gprs code in your patch communicate with a PPP stack. For example, I currently use a project where the APN is configured and a connection open with it. Then a pppd process is started to establish the IP link. It seem that you make this in a other (and certainly better) way, but I missed the big picture. Best Regards, Jean-Christian de Rivaz ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: GPRS support for Ofono
On Tue, Sep 01, 2009 at 14:02:43, Jean-Christian de Rivaz wrote: I failed to understand how the isi_gprs code in your patch communicate with a PPP stack. For example, I currently use a project where the APN is configured and a connection open with it. Then a pppd process is started to establish the IP link. It seem that you make this in a other (and certainly better) way, but I missed the big picture. The best way to do this is without PPP and this is probably what Ismo is doing. You would just create a new net_device like eth0 and hide all the ISI and cellular messaging behind that interface providing a clean IP network device. Best regards, Mikkel Christensen ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Le mardi 1 septembre 2009 23:17:36 Jean-Christian de Rivaz, vous avez écrit : Thanks for the explanation. I will start learning how phonet works. But I expect there exists a PPP stack somewhere in phonet because the modem I use don't have one. ISI modems have a PPP server-side implementation to handle USB CDC ACM and BlueTooth DUN. But there is only one instance per modem, so we reserve it for the dumb Windows PC that we may be connected to ;) And I don't think it's possible to have a IP link over GPRS without the PPP protocol between them. It might be possible in the standard, but I have not found any network offering such a PP over GPRS data service. -- Rémi Denis-Courmont http://www.remlab.net/ ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: GPRS support for Ofono
On Tue, Sep 01, 2009 at 15:17:36, Jean-Christian de Rivaz wrote: Thanks for the explanation. I will start learning how phonet works. But I expect there exists a PPP stack somewhere in phonet because the modem I use don't have one. And I don't think it's possible to have a IP link over GPRS without the PPP protocol between them. On most modern modems there is no need to have a PPP link between the Application Processor (AP) and the modem. They will often be able to transmit IP traffic directly to the AP. Hence there is no PPP stack in phonet. If your modem does not support raw IP messaging (or any other datagram service) to the AP you will probably be stuck with good old PPP. ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Everyone,
So as it happens I had also been brainstorming a GPRS API for the last several
days. And somewhat spontaneously a GPRS api discussion happened on IRC
between myself, Marcel and Ismo. The following GPRS API proposal is a result
of this discussion. I'd like all interested to comment. What needs
improvement? What is missing? What should be removed?
Please note that Secondary PDP contexts, Traffic Filters and Network Activated
(Incoming) PDP contexts are not covered in this proposal. These features are
not commonly used and none of us have real experience with them yet. However,
we considered these features and have left room in the APIs for further
expansion.
Data Connection Manager hierarchy
=
Service org.ofono
Interface org.ofono.DataConnectionManager
Object path [variable]
Methods dict GetProperties()
Returns all global system properties. See the
properties section for available properties.
Possible Errors: [service].Error.InvalidArguments
void SetProperty(string property, variant value)
Sets the property to a desired value
Possible Errors: [service].Error.InvalidArguments
[service].Error.InvalidFormat
[service].Error.Failed
void DeactivateAll()
Deactivates all active contexts.
object CreateContext()
Creates a new Primary context. Returns the object
path of the created context.
object RemoveContext()
Removes a primary context. All secondary contexts, if
any, associated with the primary context are also
removed.
Signals PropertyChanged(string property, variant value)
This signal indicates a changed value of the given
property.
Properties array{object} PrimaryContexts [readonly]
List of all primary contexts objects.
boolean Attached [readonly]
Contains whether the Packet Radio Service is attached.
The attach state might change dynamically based on
availability of network resources. If this value
changes to false, the user can assume that all
contexts have been deactivated.
If the modem is detached, certain features will not
be available, e.g. receiving SMS over packet radio
or network initiated PDP activation.
boolean RoamingAllowed [readwrite]
Contains whether data roaming is allowed. In the off
setting, if the packet radio registration state
indicates that the modem is roaming, oFono will
automatically detach and no further connection
establishment will be possible.
boolean Powered [readwrite]
Controls whether packet radio use is allowed. Setting
this value to off detaches the modem from the
Packet Domain network.
string Status [readonly]
The current packet radio registration status of a modem.
The possible values are:
unregistered Not registered to any network
registeredRegistered to home network
searching Not registered, but searching
deniedRegistration has been denied
unknown Status is unknown
roaming Registered, but roaming
uint16 LocationAreaCode [readonly, optional]
Contains the current location area code.
uint32 CellId [readonly, optional]
Contains the current network cell id.
string Technology [readonly, optional]
Contains the technology of the current network.
The possible values are: GSM, GSMCompact, UTRAN,
GSM+EGPS, UTRAN+HSDPA,
UTRAN+HSUPA,
UTRAN+HSDPA+HSUPA,
E-UTRAN
Primary Data Context hierarchy
=
Service org.ofono
Interface org.ofono.PrimaryDataContext
Object path [variable]
Methods dict
Re: GPRS support for Ofono
Hi Denis,
So as it happens I had also been brainstorming a GPRS API for the last
several
days. And somewhat spontaneously a GPRS api discussion happened on IRC
between myself, Marcel and Ismo. The following GPRS API proposal is a result
of this discussion. I'd like all interested to comment. What needs
improvement? What is missing? What should be removed?
Please note that Secondary PDP contexts, Traffic Filters and Network
Activated
(Incoming) PDP contexts are not covered in this proposal. These features are
not commonly used and none of us have real experience with them yet.
However,
we considered these features and have left room in the APIs for further
expansion.
Data Connection Manager hierarchy
=
Service org.ofono
Interface org.ofono.DataConnectionManager
I think this should be GPRSManager or something to clearly separate
between GRPS connections and actual data connection,
Object path [variable]
Methods dict GetProperties()
Returns all global system properties. See the
properties section for available properties.
Possible Errors: [service].Error.InvalidArguments
void SetProperty(string property, variant value)
Sets the property to a desired value
Possible Errors: [service].Error.InvalidArguments
[service].Error.InvalidFormat
[service].Error.Failed
void DeactivateAll()
Deactivates all active contexts.
object CreateContext()
Creates a new Primary context. Returns the object
path of the created context.
object RemoveContext()
Removes a primary context. All secondary contexts, if
any, associated with the primary context are also
removed.
I assume this is void RemoveContext(object context)
Signals PropertyChanged(string property, variant value)
This signal indicates a changed value of the given
property.
Propertiesarray{object} PrimaryContexts [readonly]
List of all primary contexts objects.
Calling this just Contexts seems to more reasonable. See comment about
interface name below.
boolean Attached [readonly]
Contains whether the Packet Radio Service is attached.
The attach state might change dynamically based on
availability of network resources. If this value
changes to false, the user can assume that all
contexts have been deactivated.
If the modem is detached, certain features will not
be available, e.g. receiving SMS over packet radio
or network initiated PDP activation.
boolean RoamingAllowed [readwrite]
Contains whether data roaming is allowed. In the off
setting, if the packet radio registration state
indicates that the modem is roaming, oFono will
automatically detach and no further connection
establishment will be possible.
boolean Powered [readwrite]
Controls whether packet radio use is allowed. Setting
this value to off detaches the modem from the
Packet Domain network.
string Status [readonly]
The current packet radio registration status of a modem.
The possible values are:
unregistered Not registered to any network
registeredRegistered to home network
searching Not registered, but searching
deniedRegistration has been denied
unknown Status is unknown
roaming Registered, but roaming
uint16 LocationAreaCode [readonly, optional]
Contains the current location area code.
uint32 CellId [readonly, optional]
Contains the current network cell id.
string Technology [readonly, optional]
Contains the technology of the current network.
The possible values are: GSM, GSMCompact, UTRAN,
GSM+EGPS, UTRAN+HSDPA,
UTRAN+HSUPA,
Re: GPRS support for Ofono
Hi Marcel,
Service org.ofono
Interface org.ofono.DataConnectionManager
I think this should be GPRSManager or something to clearly separate
between GRPS connections and actual data connection,
Two reasons for this:
- Purpose of DataConnectionManager is easier to understand for people
new to
GSM. Whereas GPRSManager really doesn't tell them anything. Everywhere else
we tend to use more easily understood terminology than what is used in the
specifications.
- GPRS is not easily Carmel-cased. E.g. we'd have to name it
GprsManager.
object RemoveContext()
Removes a primary context. All secondary contexts, if
any, associated with the primary context are also
removed.
I assume this is void RemoveContext(object context)
You're absolutely correct.
Properties array{object} PrimaryContexts [readonly]
List of all primary contexts objects.
Calling this just Contexts seems to more reasonable. See comment about
interface name below.
That sounds fine to me.
Primary Data Context hierarchy
=
Service org.ofono
Interface org.ofono.PrimaryDataContext
I would prefer if we just call this GRPSContext and explain that this is
for the primary context. The confusion between primary context and
secondary context is some GSM specific non-sense.
Should we just simply name this 'Context' and name the secondary pdp context
'SubContext'?
array{string} DomainNameServers [readonly, optional]
Holds the list of domain name servers for this
context.
What about the gateway value. In theory we can just route it to the
interface, but if the device fakes a real Ethernet interface and the
gateway is in a different subnet we need a proper host route first.
Yes, this one needs to be added. Good catch.
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
