Re: MNC/MCC as string?
Hi Jan, The attributes are really only for informational purposes only. The user would not base his decision on the mcc/mnc, but on the operator name. So before we start changing the D-Bus APIs, we need to answer these two questions: - Can a country/administrative domain have both 001 and 01 MNCs such that the use of string for the MNC is actually necessary. - What does the user find easier to use, a string or a short? I wasn't aware of this and so it might be better to just expose these as an operator id string. So we might not even split into MCC/MNC at all since it is meaning less anyway. This is probably the way to go... That said, we do want some exposure of these values since it is an easy way to determine geo location help and switch timezones etc. However I am now thinking that me might just add a Country property and do the proper translation inside oFono. Since we mostly care about these ones most. While inferring the country from MCC sounds nice, there are some corner cases where is causes problems: Digicel Bermuda uses 310/38 (or 310/038?), 310 is the US MCC: http://dougtoombs.com/2008/07/23/bermuda-the-iphone-and-att-beware-the-data-roaming/ Montenegro used MCC 220 (Serbia) before it switched to MCC 297. Wikipedia has a list: http://en.wikipedia.org/wiki/Mobile_Network_Code If you plan to ship such a list with ofono, maybe http://git.freesmartphone.org/?p=framework.git;a=blob;f=etc/freesmartphone/ogsmd/networks.tab can be useful for you. OTOH, Nokia probably has a better list. we will hide this minor defect inside the daemon and not bother the use with details. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: MNC/MCC as string?
Hi Denis, I wasn't aware of this and so it might be better to just expose these as an operator id string. So we might not even split into MCC/MNC at all since it is meaning less anyway. I'm tending to agree, which is why I wanted to start the conversation. By the way, just so that we're all clear, here's the relevant quote from the standard: MCC, Mobile country code (octet 2 and 3) The MCC field is coded as in ITU-T Rec. E212, Annex A. MNC, Mobile network code (octet 3 bits 5 to 8, octet 4) The coding of this field is the responsibility of each administration but BCD coding shall be used. The MNC shall consist of 2 or 3 digits. For PCS 1900 for NA, Federal regulation mandates that a 3-digit MNC shall be used. However a network operator may decide to use only two digits in the MNC in the LAI over the radio interface. In this case, bits 5 to 8 of octet 3 shall be coded as . Mobile equipment shall accept LAI coded in such a way. That said, we do want some exposure of these values since it is an easy way to determine geo location help and switch timezones etc. However I am now thinking that me might just add a Country property and do the proper translation inside oFono. Since we mostly care about these ones most. About the only thing that MCC/MNC is useful for is to display it during manual operator selection. MCC/MNC is not helpful at all for countries like U.S. with 4+ timezones and the same carrier id across all of them. A country property would work just as well for this. What standard do we want to use here? ITU 212? ISO 3166? or E164? Any objections from actually removing the MCC/MNC Properties? you know what. Lets remove them now. If we figure out on how to make better use of them, we can add them later. Adding properties is way simpler than deprecating/removing or changing them. For the country, I would prefer simple alpha2 encoding. That way this becomes similar to what WiFi uses for their regulatory enforcement. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
The very first oFono release
Hello everyone, I just went ahead and tagged version 0.1 of oFono. We have announced this project about 3 month ago, but the actual code base is a little bit older of course. And at some point we have to start making releases :) http://www.kernel.org/pub/linux/network/ofono/ Not everything is fully functional at this point of time, but simple voice call handling with an AT based modem (or phone simulator) should be working fine. If anything fails, please send bug reports to the mailing list and we will fix it. Basic SMS support is available and within the next releases we will make sure it works properly. After that the work on GPRS/HSPA with smooth integration into ConnMan will start. Just to give everybody a heads up. I also think it is a good time for package maintainers to look into making oFono available in their distributions. And give feedback on things that don't integrate well. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH RESEND] add zoned debug support
Hi Andres,
This adds debug flags so that when users are debugging, they can pass
arguments to --debug to specify what they want shown. --debug without
any args defaults to prior behavior.
---
include/log.h | 11 +--
src/log.c | 11 +++
src/main.c| 25 +++--
src/ofono.h |2 +-
4 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/include/log.h b/include/log.h
index 47e5ec8..8a82f13 100644
--- a/include/log.h
+++ b/include/log.h
@@ -26,6 +26,10 @@
extern C {
#endif
+typedef enum {
+ OFONO_DEBUG_CORE = 1 0,
+} ofono_debug_flags;
+
/**
* SECTION:log
* @title: Logging premitives
@@ -36,8 +40,11 @@ extern void ofono_info(const char *format, ...)
__attribute__((format(printf, 1, 2)));
extern void ofono_error(const char *format, ...)
__attribute__((format(printf, 1, 2)));
-extern void ofono_debug(const char *format, ...)
- __attribute__((format(printf, 1, 2)));
+extern void __ofono_debug(ofono_debug_flags flag, const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
+#define ofono_debug(format, ...) \
+ __ofono_debug(OFONO_DEBUG_CORE, (format), ##__VA_ARGS__)
+
I don't like this. __ofono functions should not be part of public header
files.
Instead of trying to workaround previous users, just fix the users and
provide the default zone for DBG().
/**
* DBG:
diff --git a/src/log.c b/src/log.c
index 273e3ba..167fe21 100644
--- a/src/log.c
+++ b/src/log.c
@@ -29,6 +29,7 @@
#include ofono.h
static volatile gboolean debug_enabled = FALSE;
+static guint debug_flags;
/**
* ofono_info:
@@ -67,7 +68,8 @@ void ofono_error(const char *format, ...)
}
/**
- * ofono_debug:
+ * __ofono_debug:
+ * @flag: zone flag (ie, OFONO_DEBUG_CORE)
* @format: format string
* @varargs: list of arguments
*
@@ -76,11 +78,11 @@ void ofono_error(const char *format, ...)
* The actual output of the debug message is controlled via a command line
* switch. If not enabled, these messages will be ignored.
*/
-void ofono_debug(const char *format, ...)
+void __ofono_debug(ofono_debug_flags flag, const char *format, ...)
{
Why not just using unsigned long here. I don't like the idea of
overloading enum with bitmask here.
Actually the more I think about it, do we wanna have multiple zones per
debug message. That sounds way to complicated anyway. So why not just
make ofono_debug_flags a simple enum and then use the bitmask only
internally.
va_list ap;
- if (debug_enabled == FALSE)
+ if (!debug_enabled || !(debug_flags flag))
return;
va_start(ap, format);
@@ -98,7 +100,7 @@ void __ofono_toggle_debug(void)
debug_enabled = TRUE;
}
-int __ofono_log_init(gboolean detach, gboolean debug)
+int __ofono_log_init(gboolean detach, gboolean debug, guint dflags)
{
int option = LOG_NDELAY | LOG_PID;
@@ -110,6 +112,7 @@ int __ofono_log_init(gboolean detach, gboolean debug)
syslog(LOG_INFO, oFono version %s, VERSION);
debug_enabled = debug;
+ debug_flags = dflags;
This is double. If dflags are not set, then don't enable debug. No need
to provide two parameters that do the same.
Also dflags is a pretty weird variable name.
return 0;
}
diff --git a/src/main.c b/src/main.c
index 7542e13..7227bde 100644
--- a/src/main.c
+++ b/src/main.c
@@ -54,12 +54,33 @@ static void system_bus_disconnected(DBusConnection *conn,
void *user_data)
static gboolean option_detach = TRUE;
static gboolean option_debug = FALSE;
+static guint debug_flags = 0;
+
+static GDebugKey keys[] = {
+ { core, OFONO_DEBUG_CORE },
+};
+
+static gboolean parse_debug_flags(const gchar *option_name, const gchar
*value,
+ gpointer data, GError **err)
+{
+ option_debug = TRUE;
+
+ /* NULL means no string was supplied to --debug. We default to core
+ * in that scenario; perhaps we should be defaulting to all instead?
*/
+ if (!value)
+ value = core;
+
+ debug_flags = g_parse_debug_string(value, keys,
+ sizeof(keys) / sizeof(keys[0]));
+ return TRUE;
+}
static GOptionEntry options[] = {
{ nodetach, 'n', G_OPTION_FLAG_REVERSE,
G_OPTION_ARG_NONE, option_detach,
Don't run as daemon in background },
- { debug, 'd', 0, G_OPTION_ARG_NONE, option_debug,
+ { debug, 'd', G_OPTION_FLAG_OPTIONAL_ARG,
+ G_OPTION_ARG_CALLBACK, parse_debug_flags,
Enable debug information output },
{ NULL },
};
@@ -109,7 +130,7 @@ int main(int argc, char **argv)
}
#endif
- __ofono_log_init(option_detach, option_debug);
+
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 Remi, Our current assumption is that the basic setup of IP address, netmask and broadcast are done by oFono. Only routing and DNS are up to other programs like ConnMan for example. WHAAT? No way. There is just no way. We need to support letting the calling program configure the routing parameters manually. For instance, if we want to connect to multiple primary access points, it simply won't work if Ofono configures everything. Instead, we need to either setup source routing or separate network name-spaces. Ofono does not know what the caller intends to do with the APN (and should not need to), so it cannot configure IP parameters. Besides, it makes very little if any sense to configure IP parameters but not DNS. personally I don't care who finally sets the IP for the interface. It really just makes no difference. What problem do you see if oFono would set the IP address on that newly created interface that is up and running now? My point is that gateway and DNS is out of the question for oFono. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Hi Ismo, of this discussion. I'd like all interested to comment. What needs improvement? What is missing? What should be removed? Here are some comments. Some of the comments were already present in the IRC discussion, but I'll repeat them here anyway. First of all, the both Denis's proposal and mine look quite much the same -- the basic objects are the GPRS manager and the PDP contexts, and they offer quite the same functionality. Please correct me if I understood some parts of Denis's proposal incorrectly. The big differences between the API proposals are the creation and connecting of the contexts and the handling of the attached/detached state. I'll try to address the both issues below. In this proposal the contexts PDP contexts are persistent on the device, meaning that they stay there also when Ofono is shut down. The idea is that the contexts are made to Ofono by the GPRS provisioning programs or by the UI, and then ConnMan or another upper layer needs just to activate the context to actually do the connection. In my proposal I thought that the higher layer components would keep track of the access points and call the Connect() method with the suitable connection parameters. This is a philosophical difference from the provisioning point of view. An operator will likely support multiple configured access points, and the provisioning program should know their purpose from the configuration file (internet/MMS/WAP/...). This metadata is not there in the Ofono context, meaning that the provisioning program needs to store somewhere else the mapping between Ofono context object paths and the neccessary metadata. This begs the question: why not store all connection data (APN, authentication data, other metadata) to this external store in the first place? I mentioned that already. We do wanna store something like Type that says internet, mms, wap etc. The only comment here was that for network initiated context we have no idea of its intent. The attachment to the GPRS network in this proposal is bit vague. To my understanding, to attach the GPRS you need to set Powered property on and RoamingAllowed on. To detach you need to set the Powered property off. Since attaching can take a long time (or fail), does this mean that the attach errors are handled in the Powered property setter callback? Or what happens when (during roaming) the Powered is already on and the user puts the RoamingAllowed on -- where are the attach errors handled? I kind of think that it might be a better idea to just expose the GPRS registration status and the attach status, and let a higher level component explicitly set the attach/detach with either a readwrite property or Attach() and Detach() methods. Powered = on and RoamingAllowed = yes and Roaming = true will lead to auto-attach. Powered = on and RoamingAllowed = no and Roaming = true will lead to detach. Powered = on and Roaming = false will lead to auto-attach. Powered = off will lead to detach. We are not going to expose Attach() or Detach() method. As explained there are global anyway and so pointless to expose. This can be handled in the background without bothering the higher level application with these details. In my proposal the Status variable was a three-state variable, having the states attached, detached and suspended. Suspended was specified to mean that the GPRS was attached, but temporarily not available (for instance because of a 2G phone call or temporary loss of cellular connectivity). In the IRC discussion someone said that this property would not be part of Denis's API because the tri-state variables were bad and the suspended status was not supported by spec 27.007. However, the problem is that Maemo 5 already has UI support for indicating that the connection is suspended, and that makes it harder to drop the feature. Could the suspended status enum still be left there, and just have the modems that don't support the feature to never go to suspended mode? I am still not seeing the point in what suspended will do for us and the UI. And that Maemo 5 exposed such a state in the UI is not an argument to keep doing it. I asked this before, what are we suppose to be doing when we get signaled suspended? Still one difference to my proposal was the dropping of the TX and RX byte counts, and the explanation that those statistics would be handled by the upper layers by reading the values from the network interface. I mentioned the problem where the connection was terminated by the network and the network interface was brought down before the upper layer had a chance to read the values. After giving the matter some thought, I still feel that for this reason the modem driver would need to get the TX/RX data and send it to the upper levels, if at all possible. Since many operators charge by the amount of data transferred, not losing the TX/RX data is quite crucial. At least Nokia phones
Re: GPRS support for Ofono
Hi Aki, Powered = on and RoamingAllowed = yes and Roaming = true will lead to auto-attach. Powered = on and RoamingAllowed = no and Roaming = true will lead to detach. Powered = on and Roaming = false will lead to auto-attach. Powered = off will lead to detach. Why is auto-attach dependent on roaming? power consumption. No need to attach if you don't wanna allow data roaming. We are not going to expose Attach() or Detach() method. As explained there are global anyway and so pointless to expose. This can be handled in the background without bothering the higher level application with these details. AFAIK, attach status of GPRS has both regulatory aspects to consider (some operators require always attached vs. some prohibit it) as well as power consumption issues (auto-attach might draw more power than on-demand, although there's an inverse effect on latency). These are issues you need to take into account, and higher layers in the stack definitely *need* to be aware of as well. Also, I don' t think oFono is the correct place for these decisions - better expose a necessary API and let upper layers deal with it. I disagree. We can have a config option that always attaches or attached on demand, but this should not be exposed to any higher level applications. In general, I appreciate the attempt to hide ugly details from applications, but unfortunately some things can't well be hidden. Another unrelated, but similar issue is network scanning. It just isn't going to work without us exposing it in the D-Bus API explicitly. The reason is simple, Nokia modems suspend GPRS when scanning (or registering), simply because the operation will take roughly three times as long with GPRS attached. You will find this feature in current phones, too. Now, there is no way we can have GPRS suspend without warning just because the stack deems it necessary to scan for networks. Again the intention is good, but the end result not so good. I don't want to start patching oFono to support this type of basic stuff. I mentioned this before. We might need to add a config option to allow integrators choose different behavior. In my proposal the Status variable was a three-state variable, having the states attached, detached and suspended. Suspended was specified to mean that the GPRS was attached, but temporarily not available (for instance because of a 2G phone call or temporary loss of cellular connectivity). In the IRC discussion someone said that this property would not be part of Denis's API because the tri-state variables were bad and the suspended status was not supported by spec 27.007. However, the problem is that Maemo 5 already has UI support for indicating that the connection is suspended, and that makes it harder to drop the feature. Could the suspended status enum still be left there, and just have the modems that don't support the feature to never go to suspended mode? I am still not seeing the point in what suspended will do for us and the UI. And that Maemo 5 exposed such a state in the UI is not an argument to keep doing it. I asked this before, what are we suppose to be doing when we get signaled suspended? You will find that practically every Nokia phone behaves this way, i.e., you make a call in 2G, and the UI will inform you that GPRS is suspended while on call. So what is the difference signaling a disconnect and re-connect to the application? Still one difference to my proposal was the dropping of the TX and RX byte counts, and the explanation that those statistics would be handled by the upper layers by reading the values from the network interface. I mentioned the problem where the connection was terminated by the network and the network interface was brought down before the upper layer had a chance to read the values. After giving the matter some thought, I still feel that for this reason the modem driver would need to get the TX/RX data and send it to the upper levels, if at all possible. Since many operators charge by the amount of data transferred, not losing the TX/RX data is quite crucial. At least Nokia phones have a GPRS data counter feature, that is supposed to show the transferred data. This said, I know that not all modem drivers can get the data from a connection that has been terminated by the network, but that does not mean that the modem drivers that can get the data should just discard it. This is for ConnMan or similar to figure out. And we can always count via netfilter or some other facility. Counting inside oFono makes no sense. However we could send out a statistics signal before taking the interface down if it would be really needed. Making it part of the properties and having to poll for it is wrong. I believe emitting a signal was Ismo's original proposal. I will talk with Stephen and David about this and what it a proper way to collect
Re: GPRS support for Ofono
Hi Remi, I mentioned that already. We do wanna store something like Type that says internet, mms, wap etc. The only comment here was that for network initiated context we have no idea of its intent. And we do not want to. The attachment to the GPRS network in this proposal is bit vague. To my understanding, to attach the GPRS you need to set Powered property on and RoamingAllowed on. To detach you need to set the Powered property off. Since attaching can take a long time (or fail), does this mean that the attach errors are handled in the Powered property setter callback? Or what happens when (during roaming) the Powered is already on and the user puts the RoamingAllowed on -- where are the attach errors handled? I kind of think that it might be a better idea to just expose the GPRS registration status and the attach status, and let a higher level component explicitly set the attach/detach with either a readwrite property or Attach() and Detach() methods. Powered = on and RoamingAllowed = yes and Roaming = true will lead to auto-attach. That's just _idiotic_ from the naming perspective. A modem can have radio on or off. Whether this is done by completely powering the modem off, or by going into some kind of flight mode, is driver-specific. Hence the powered name is semantically wrong. When possible, it's actually best to keep the modem in flight mode, so that e.g. the SIM is still usable. The story is basically the same with roaming. Roaming means you are outside your home network. It does not mean that you want to auto-attach or not. Some people _never_ want to auto-attach, and some people _always_ want to auto-attach. In fact, different operators have different requirements with that regard. Sure, some people want to auto-attach if and only if not roaming. Given that roaming is not just about GPRS, the name is wrong and confusing. But more importantly, we need t support turning the radio on while in the home network yet _not_ attach automatically. This has operator requirements as well as power saving implications. you did read the API docs Denis send around? These are not global Powered and RoamingAllowed properties. They are specific to the GPRS Manager and thus have specific meaning in that context. It has nothing to do with flight mode or radio off. And for the different operator policy/behavior, that should be configured via config option and not via D-Bus. The integrator should make a choice. It it is a list of mappings of operator = attach policy, then that is also fine. We are not going to expose Attach() or Detach() method. And we are going to expose it. And for what good is this. So we have two applications fighting about Attach and Detach and some weird policy somewhere upper in the stack with no real sense on what is happening. Sorry but I am not buying this at all. If the only argument is that different providers require different auto-attach policies then we have that as described via a config option and be done with it. Otherwise I like to see real examples where something like ConnMan, an MMS application or any other application need control over the attach status. I am still not seeing the point in what suspended will do for us and the UI. And that Maemo 5 exposed such a state in the UI is not an argument to keep doing it. I asked this before, what are we suppose to be doing when we get signaled suspended? User, as well as intelligent (connectivity-aware) applications, need to know about this so that they understand why Internet is momentarily broken. It's as simple as that. Oh we could also use the network device carrier flag, and then use Netlink (and we probably should do that too), but we all know how horrible Netlink is from userland. Using IFF_RUNNING and IFF_LOWER_UP sounds more reasonable then some suspended state. (...) This is for ConnMan or similar to figure out. And we can always count via netfilter or some other facility. Counting inside oFono makes no sense. However we could send out a statistics signal before taking the interface down if it would be really needed. Making it part of the properties and having to poll for it is wrong. This has legal(ish) implications related to charging. Skipping this is not exactly an option (for a device vendor). I actually agree that this is ugly in some ways. In theory, I don't really care if Ofono or the over-lying connection manager takes care of it. *But* we even need to collect statistics for contexts not handled in Ofono (e.g. Windows PC tethering). And I doubt that Connman would be able to do that properly. It's ugly and annoying, but we have to suck it up. As I mentioned it before. We figure something out. We do have the same problem for all hotplug network devices. Especially with hotplug 3G dongles, we need to do something better than counting by ourself. The kernel should do it for us. If you yank the dongle,
Re: GPRS support for Ofono
Hi Aki, AFAIK, attach status of GPRS has both regulatory aspects to consider (some operators require always attached vs. some prohibit it) as well as power consumption issues (auto-attach might draw more power than on-demand, although there's an inverse effect on latency). These are issues you need to take into account, and higher layers in the stack definitely *need* to be aware of as well. Also, I don' t think oFono is the correct place for these decisions - better expose a necessary API and let upper layers deal with it. I know you guys are coming from mobile phone perspective, but that one isn't the only scenario. We have netbooks/laptops with gprs data cards to consider as well. Hardcoding operator requirements is also not an option, since SIM cards can be changed. I fundamentally disagree that the logic should be in the higher layers. oFono has all the information to handle this, and can be hinted on what is appropriate behavior easily enough. oFono should simply store the auto-attach preferences in its IMSI-indexed preference store. The operator can bootstrap these preferences if required. Operator requirements were only half of the story. You need to be able to disable auto-attach in low power conditions, and this logic definitely doesn't belong in oFono. This applies equally well to mobile phones as it does to laptops/netbooks. this a different requirement. Where do you get your low power information from? I would prefer that we add a plugin that gets informed of this low power situation and then is able to adjust such things. I am against exposing this in user application/UI API. I am with you that such requirements have to be met, but not via the D-Bus API. So these kind of policy details are device/manufacturer specific and having a custom plugins that can be enabled by the integrator sounds best to me if a config file is not flexible enough. In general, I appreciate the attempt to hide ugly details from applications, but unfortunately some things can't well be hidden. Another unrelated, but similar issue is network scanning. It just isn't going to work without us exposing it in the D-Bus API explicitly. Modem drivers will have some control over whether the scan is performed automatically, but denying the capability for everyone is not the right thing to do either. Agree, and I don't think that was what I was suggesting either. The reason is simple, Nokia modems suspend GPRS when scanning (or registering), simply because the operation will take roughly three times as long with GPRS attached. You will find this feature in current phones, too. Then ideally you should have two scan modes, periodic and user initiated. For periodic mode we don't care how long it takes. +1 Sounds like a plan. So we do need an /etc/ofono/main.conf :) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: GPRS support for Ofono
Hi Waldo, I am still not seeing the point in what suspended will do for us and the UI. And that Maemo 5 exposed such a state in the UI is not an argument to keep doing it. I asked this before, what are we suppose to be doing when we get signaled suspended? User, as well as intelligent (connectivity-aware) applications, need to know about this so that they understand why Internet is momentarily broken. It's as simple as that. +1 they need to know if they can do something useful with this information. If not, then this state is just pointless. However the D-Bus API is the wrong location for this detail. Oh we could also use the network device carrier flag, and then use Netlink (and we probably should do that too), but we all know how horrible Netlink is from userland. Using IFF_RUNNING and IFF_LOWER_UP sounds more reasonable then some suspended state. IF_OPER_DORMANT seems a more accurate reflection of the state. Yep. That sounds good. We don't use that in ConnMan right now, but that can be fixed of course. I still need to figure out what we are suppose to do when this gets signaled to us. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Hi Remi, Actually this one is missing from the API proposal. Marcel already wanted the context type (internet, mms, wap, etc) information. I've updated the proposed API in git. This is not going to work. Depending on the operator, you may have more than one type for a single context (e.g. WAP+MMS), or worse, multiple contexts of the same type (e.g. Internet with only Web and Internet with full IP). we might need some extra work here and define what we want. I am confident that we can break this down to something useful. Worst case we something free form. As discussed previously, we want oFono to manage this data, since it can do this by using the IMSI. So if you insert a different operator SIM your APN settings are magically updated for that operator. I have a feeling this does not work either. If I upgrade my subscription, the APN may change while not the IMSI, no? We can update these details at any time. Storing them on a per ISMI basis is a good idea and something new that oFono does (as far as what I have seen). So if you switch SIM cards, then you don't have to worry about anything when traveling. It does the right thing you configured. Especially with the devices like the Nokia netbook and the iPhone where you can easily switch SIM cards without power on/off this will come in handy. If an upgrade of the subscription needs manual interaction from the user or an operator message to update the settings, then that is fine. Since the user did something to begin with. So they are aware of that something changed (hope the operator told them). While just switching a SIM card and by accident using a wrong APN and causing expensive roaming would be a worse thing. Especially since there is an expectation from the user when switching SIM cards. In my proposal the Status variable was a three-state variable, having the states attached, detached and suspended. Suspended was specified to mean that the GPRS was attached, but temporarily not available (for instance because of a 2G phone call or temporary loss of cellular connectivity). In the IRC discussion someone said that this property would not be part of Denis's API because the tri-state variables were bad and the suspended status was not supported by So the general rule is that if a feature isn't explicitly allowed in 27.007, we do not support it. There are exceptions to this of course. 27.007 only supports two states for context: activated and deactivated. Our general rule is that if we have a UI requirement, or better an actual use case for it, we support it. Suspended GPRS fits both. Can you share these UI requirements with us. spec 27.007. However, the problem is that Maemo 5 already has UI support for indicating that the connection is suspended, and that makes it harder to drop the feature. Could the suspended status enum still be left there, and just have the modems that don't support the feature to never go to suspended mode? I'd rather not. If this feature is really required, then some sort of heuristic can be applied. (e.g. Powered on, Attached on, but context is deactivated most likely means temporary unavailability of resources) Is this some kind of joke? Don't you know the difference between pause and stop on your MP3 player and between suspend to memory and power off on your computer? Second guessing does not fit in my definition of easy to use self- documenting for an API. This really belong in the kernel. Only the kernel can reliably know when a network interface has been brought down and notify the interested applications with the statistics. You're missing the point. Yes, any body can extract the statistics for a running context. But data counters are cumulating. To compute the sum properly, there are but two options: # Either the GPRS middleware requests kernel per-interface statistics right before destroying the interface, and sums with the earlier total. # Or the modem does it internally. There is no way some random userland interface application can do it just by requesting the kernel, since it cannot know when to request those statistics. So my 2 cents here are that whatever so modem does for statistics it should match the details its exports via the networking interface. And on the kernel part and destroyed interface, as I mentioned, we need to discuss this with the kernel guys. I wanna get an answer what can be done to help us. We have most kernel guys at netconf in Portland in three weeks and I gonna check with them. Nokia hardware supports this explicitly, but many others don't. That's irrelevant. Hardware support helps in the sense that it can include statistics for contexts external to Ofono. If the hardware does not support it, then it needs to be done manually in the GPRS middleware. That won't make the requirement go away. What do you expect middleware to do exactly. Our
RE: GPRS support for Ofono
Hi Waldo, I know why you want this, but I'm still against the counter being an oFono driver API. There needs to be a proper kernel interface that signals the application when an interface has gone away with the rx/tx details. This way we handle this generically for all modems without relying on some intrinsic hardware capabilities. This still doesn't solve the case where the modem is accessed from a PC client and forwards PPP data as that data will not go through any network interface, e.g. BT DUN or USB tethering. The cases you describe imply that oFono is not even in control of the gprs context. How would we track/report the tx/rx statistics in that case? It's probably difficult if the PC client is allowed to redefine GPRS contexts, but otherwise oFono should at least be able to report the total tx/rx for the context's it has defined. The BT DUN / USB bridge could call into oFono and trigger a poll of all the stats to update them, e.g. when a BT DUN connection is disconnected. how should it do this if oFono is not in the mix. If you are using Bluetooth DUN and point it to a virtual TTY, then you are out of look. If using USB CDC ACM then same applies. The real solution here is Bluetooth PAN and USB CDC Ether which do properly interact with the networking stack. The modem is really in the best position to provide the most reliable information on data usage. You can still use statistics from the network interfaces as a fall-back in case the modem can not provide this information. I don't disagree, but not every modem can track these statistics and this isn't described by 27.007. I suggest the initial support should be enabled by the modem driver implementing a custom D-Bus interface and expose these details however it wishes. The modem driver has no desires of its own :-) It really comes down to what the UI needs and I doubt that the UI wants to deal with this on a modem by modem basis, but sure it's a possibility. The current consent is that we might send a one time signal with all statistics details once the interface goes away. Or we can make the kernel help us here. However I prefer that we sit this out a little and play with what is possible before knocking down an API. I am not even sure what different hardware would give us and how things need to work. I don't see us having enough information to understand what is needed from a hardware, driver or oFono core point of view. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Hi Denis, The reason is simple, Nokia modems suspend GPRS when scanning (or registering), simply because the operation will take roughly three times as long with GPRS attached. You will find this feature in current phones, too. Then ideally you should have two scan modes, periodic and user initiated. For periodic mode we don't care how long it takes. +1 Sounds like a plan. So we do need an /etc/ofono/main.conf :) So my current thinking is to introduce a method to NetworkRegistration property called 'ProposeOperatorScan' and a new property called 'OperatorScanInterval'. The driver api can then support a (non-standard *gasp*) option to say whether this scan was initiated actively or not. Maybe the Nokia modems can use this information not to drop the GPRS link if it isn't an active scan being performed. I really prefer if we put this into /etc/ofono/main.conf since the UI should not configure this at all. It is more a one time setting based on what the integrator expects as a behavior. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Hi Remi, It's probably difficult if the PC client is allowed to redefine GPRS contexts, but otherwise oFono should at least be able to report the total tx/rx for the context's it has defined. The BT DUN / USB bridge could call into oFono and trigger a poll of all the stats to update them, e.g. when a BT DUN connection is disconnected. how should it do this if oFono is not in the mix. If you are using Bluetooth DUN and point it to a virtual TTY, then you are out of look. If using USB CDC ACM then same applies. The real solution here is Bluetooth PAN and USB CDC Ether which do properly interact with the networking stack. When we have patched all the Windows PC of the world, we can consider it. my approach would be to try and see how far we get this pushing towards PAN and CDC Ether :) In the mean time, AT+PPP emulation is required. Some modems do provide data counters including that. I've seen it as a requirement that I would rather have avoided but could not. It's ugly and arguably stupid, but required anyway. In fact, if Ofono won't do it, ConnMan will have to, which is probably by all means worse. Actually it is worth for high-speed modems with a network interface and no internal PPP emulation, we would have to do that emulation somewhere in the host stack. And now my brain hurts :( You can argue that it should be a driver-specific feature, but it has to be there. Hence, I would guess that more than one driver will support eventually, at which point it should probably be in the common API. We have to figure something out, but right I would prefer if we GPRS up and running and after that tackle the statistics part. Since without GPRS, we don't need the statistics ;) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] G1: Add initial HTC G1 modem support
Hi Andreas, 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. --- Makefile.am |3 + plugins/g1.c | 182 ++ 2 files changed, 185 insertions(+), 0 deletions(-) create mode 100644 plugins/g1.c patch has been applied. I did have a trailing whitespace mistake, but I think git was just being paranoid. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] G1: Add a G1 syntax for parsing
Hi Andreas, 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. --- plugins/g1.c | 87 - 1 files changed, 85 insertions(+), 2 deletions(-) this patch has been applied now, too. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] G1: Add an SMS quirk for CNMI mode
Hi Andres, The G1 doesn't support mode2, despite advertising it. The G1 chokes w/ an Error 303 when we specify NMI mode 2. Adding a quirk to drop that mode from the supported list (just use mode 1) allows the G1 to properly deal with SMS. --- drivers/atmodem/sms.c| 19 ++- drivers/atmodem/vendor.h |1 + plugins/g1.c |3 +++ 3 files changed, 18 insertions(+), 5 deletions(-) patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Comments about ofono-0.4 release
Hi guys, since we had so much movement in the source code, I went ahead and tagged a new version. So welcome our 0.4 release. The internal modem driver and feature frameworks got a whole re-write, but I am not going into that details here. Go and join the IRC if you really care about it ;) With this release the generic_at driver is gone and replaced by modem configuration driver and a separate one for phone simulator. So the file you wanna look at is /etc/ofono/modem.conf now. It lets you specify various modems and which driver they are suppose to be used. The file is pretty self explanatory. In addition the phone simulator driver, we now have a working driver for the Android/HTC G1 devices. And an initial driver for the Ericsson MBM laptop cards that at least adds support for SMS text messaging. The daemon can also now be built with udev support. This is by default enabled, but can be disabled if required or the target platform doesn't use udev. The udev support will extend the modem configuration file and allow for dynamic modem configuration. We are not there yet at this point of time, but should be getting there pretty soon. And as an extra goody, I converted to a full non-recursive build system which should speed up everything and also allow the linker to do a better job in stripping unused symbols. The biggest advantage of course is when using automake-1.11, it looks way more sexy now :) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Support for GSM data cards
Hi guys, so I started working on udev integration and auto-detection and pushed a few new drivers into the upstream oFono repository. It know has drivers for Android/HTC G1, Ericsson MBM, Option HSO, Novatel and Huawai modems. An example of three devices attached to the same system would look then like this: [ /mbm2 ] Powered = 1 Interfaces = org.ofono.SmsManager org.ofono.NetworkRegistration Model = F3507g Manufacturer = Ericsson Serial = 0115xx Revision = R1B003 [ org.ofono.SmsManager ] ServiceCenterAddress = +17057969300 [ org.ofono.NetworkRegistration ] Status = registered Operator = ROGERS AvailableOperators = /mbm2/operator/302720 LocationAreaCode = 65080 CellId = 1305559 [ /huawei1 ] Powered = 1 Interfaces = org.ofono.NetworkRegistration Model = E160 Manufacturer = huawei Serial = 3538xx Revision = 11.604.18.01.00 [ org.ofono.NetworkRegistration ] Status = roaming Operator = CAN Rogers Wirel AvailableOperators = /huawei1/operator/302720 [ /hso0 ] Powered = 1 Interfaces = org.ofono.SmsManager org.ofono.NetworkRegistration Model = GlobeTrotter HSDPA Modem Manufacturer = Option N.V. Serial = 3597xx Revision = 2.5.13Hd (Date: Feb 18 2008, Time: 18:32:40) [ org.ofono.SmsManager ] ServiceCenterAddress = +49171076 [ org.ofono.NetworkRegistration ] Status = roaming Operator = Rogers Wireless - T-Mobile D AvailableOperators = /hso0/operator/302720 LocationAreaCode = 16100 CellId = 47738 All of the drivers are far from finished. The best working one is the Ericsson MBM right now. I even managed to send a text message via the SMS interface. When trying that with an Option device the AT parser hangs itself. So we need some quirk there. Everybody is welcome to debug it and send a patch. The Novatel and Huawei devices, don't seem to support SMS. Or they do and don't report it correctly. Also the Novatel card has problems with the network registration. It refuses to work. This needs also to be debugged. If the drivers network registration does work, then it looks nicely like the following output: [ /mbm2 ] [ /mbm2/operator/302720 ] Status = current MobileNetworkCode = 720 Technology = UTRAN Name = ROGERS MobileCountryCode = 302 [ /huawei1 ] [ /huawei1/operator/302720 ] Status = current MobileNetworkCode = 720 Technology = UTRAN Name = CAN Rogers Wirel MobileCountryCode = 302 [ /hso0 ] [ /hso0/operator/302720 ] Status = current MobileNetworkCode = 720 Technology = GSM Name = Rogers Wireless - T-Mobile D MobileCountryCode = 302 The MBM device contains a local SIM card and the other two are actually roaming. The goal for these data cards should be to support text messaging and GPRS (once we defined and implemented that interface). Any help is more than welcome. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: failed to start ofonod....
Hi Lihan, I also face the problem when I install ofono on a new machine. Unable to hop onto D-Bus I run as root, but the same error as WuYongbo and no configure directory “ofono” under /etc By the way, I manually installed dbus-1-dev and dbus-glib-1-dev, unless ./bootstrap-configure reminds that DBUS=1.0 is required. you need to copy the D-Bus security policy before it will work. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [Patch]Unref GIOChannel in g_at_chat_new_from_tty
Hi Zhenhua, Attached is the patch to unref GIOChannel in g_at_chat_new_from_tty. If the GAtChat is created from g_at_chat_new, the reference count of channel is increased by g_io_add_watch_full(channel), so we need to unref it. If GAtChat is failed to create, we also need to unref channel to free it. patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Autotool improvements.
Hi guys, Bootstrap script now tries to use explicitly versioned automake, which makes life easier with scratchbox and friends. Bootstrap also uses autoreconf instead of explictly running various autotools. As autoreconf already uses those variables, I wonder why not just call it and be done with it. when I started using the bootstrap stuff we use accross BlueZ, ConnMan, oFono etc. the autoreconf was horrible and broken. Let me look into this again and see how we can make this more consistent. Especially since everybody should use automake-1.11 do get the sexy silent build system. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Replace Glib type with standard C type
Hi Denis, Below patch is to replace Glib type with standard C type in hfpmodem.h, like guint, guint8, etc. Patch has been applied. Thanks. please double check the author name is in the right format. Firstname Lastname and not Lastname, Firstname what Exchange always messes up. Git will take it literally :( Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] Added functionality to remove modem from system, if needed.
Hi Ryan,
---
test/enable-modem |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/test/enable-modem b/test/enable-modem
index 0f9f604..d44783d 100755
--- a/test/enable-modem
+++ b/test/enable-modem
@@ -1,6 +1,7 @@
#!/usr/bin/python
import dbus
+import sys
bus = dbus.SystemBus()
@@ -14,4 +15,7 @@ path = properties[Modems][0]
modem = dbus.Interface(bus.get_object('org.ofono', path),
'org.ofono.Modem')
-modem.SetProperty(Powered, dbus.Boolean(1))
+if len(sys.argv) 1 and sys.argv[1] == '0' :
+modem.SetProperty(Powered, dbus.Boolean(0))
+else :
+modem.SetProperty(Powered, dbus.Boolean(1))
if you want this, then add a disable-modem script.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: Patch on unsupported AT command
Hi Yang, If some unsupported AT command is issued, different modem may have their own response. Now at my hand is a Huawei modem (EM770W), and it returns COMMAND NOT SUPPORT. In my case, this modem doesn't support AT+CGAUTO=0 in atmodem/gprs.c. Current oFono will hang there for it's not a valid return. We may have some quirk to handle this problem, the same way as current code in network-registration.c with CALYPSO. But I wonder if it's better to add the response string into terminator table, so that we don't need this kind of quirk here and there. I'm not sure if this is the better/best way to handle this problem. After all, the table may become larger and larger is more and more specific terminator like this are added. Comments are welcome! this lovely broken Huawei modem where the firmware developers were incapable of reading the specification and just made up a new response. I think this might need a GAtChat quirk function where we can add extra terminator responses that will be recognized. And maybe even translated into something meaningful. Denis, or do you want this quirked in every plugin or modem driver? Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Patch on unsupported AT command
Hi Denis, I think this might need a GAtChat quirk function where we can add extra terminator responses that will be recognized. And maybe even translated into something meaningful. Originally I had the terminators freely definable on the GAtChat + some hardcoded ones, but abandoned that approach since it didn't seem useful. Perhaps this needs to be resurrected. Denis, or do you want this quirked in every plugin or modem driver? I do prefer non-standard terminators to be setup in the plugin/driver. I meant adding something like g_at_chat_add_terminator() or so. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Patch on unsupported AT command
Hi Yang, I think this might need a GAtChat quirk function where we can add extra terminator responses that will be recognized. And maybe even translated into something meaningful. Originally I had the terminators freely definable on the GAtChat + some hardcoded ones, but abandoned that approach since it didn't seem useful. Perhaps this needs to be resurrected. Denis, or do you want this quirked in every plugin or modem driver? I do prefer non-standard terminators to be setup in the plugin/driver. I meant adding something like g_at_chat_add_terminator() or so. Yes, that is what I meant as well. Attached is the patch to support plugin specific terminator, and it supports Huawei's special one COMMAND NOT SUPPORT. you need to split this into a GAtChat specific patch and oFono plugin patch. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Patch on unsupported AT command
Hi Denis, +g_at_chat_add_terminator(chat, +EXT ERROR:, 11, FALSE); +g_at_chat_add_terminator(chat, +CME ERROR:, 11, FALSE); +g_at_chat_add_terminator(chat, +CMS ERROR:, 11, FALSE); +g_at_chat_add_terminator(chat, NO ANSWER, -1, FALSE); +g_at_chat_add_terminator(chat, CONNECT, -1, TRUE); +g_at_chat_add_terminator(chat, NO CARRIER, -1, FALSE); +g_at_chat_add_terminator(chat, BUSY, -1, FALSE); +g_at_chat_add_terminator(chat, NO DIALTONE, -1, FALSE); +g_at_chat_add_terminator(chat, ERROR, -1, FALSE); +g_at_chat_add_terminator(chat, OK, -1, TRUE); I really don't like this. Lets keep the non-standard terminators in a separate list. I don't want the vast majority of the drivers incurring the cost of multiple g_new/g_frees. I have to agree on this. We should keep the penalty for well behaving cards as small as possible. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Palm Pre modem plugin
Hi Morphis, I saw you added some time ago support for the Palm Pre MSM Modem. I am working on the FSO (http://www.freesmartphone.org/) side to support this kind of MSM modem as well. I am are currently stucked with the binary protocol spoken between modem and userland in webOS. The Problem why we do this is the following: As I and some other people find out, there is no support on the data channel (the one you use in oFono to speak plain AT with the modem) for unsolicited responses. So the only option is to support the binary protocol to get minimally the response from the modem when for example a call arrives. So my question is: How do you want to manage this with your plugin for the Palm Pre modem? Do you even plan to use the serial interface on the /dev/modemuart port? I expect that at some point someone does a MSM plugin that talks this binary protocol and it will work with more Qualcomm based devices than the Palm Pre. We will see. However any patches are more than welcome. No patches from me for oFono as I am a FSO fan boy :) I think we need to convert you :) To be honest it would be way simpler for you to write a MSM plugin for oFono since it is actually designed to support binary protocols like the Nokia Phonet in the first place. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Palm Pre modem plugin
Hi Morphis, I saw you added some time ago support for the Palm Pre MSM Modem. I am working on the FSO (http://www.freesmartphone.org/) side to support this kind of MSM modem as well. I am are currently stucked with the binary protocol spoken between modem and userland in webOS. The Problem why we do this is the following: As I and some other people find out, there is no support on the data channel (the one you use in oFono to speak plain AT with the modem) for unsolicited responses. So the only option is to support the binary protocol to get minimally the response from the modem when for example a call arrives. So my question is: How do you want to manage this with your plugin for the Palm Pre modem? Do you even plan to use the serial interface on the /dev/modemuart port? I expect that at some point someone does a MSM plugin that talks this binary protocol and it will work with more Qualcomm based devices than the Palm Pre. We will see. However any patches are more than welcome. No patches from me for oFono as I am a FSO fan boy :) I think we need to convert you :) You can try but I think it's currently impossible :) for oFono we have a pretty much brought spectrum and if the Qualcomm GOBI cards use a similar protocol to what we find in the Palm Pre, then you might wanna actually consider going for oFono. Since we will be pushing oFono as the main telephony stack for desktops, laptops and netbooks. Using it on actual mobile phones is just a side product. And I did see FSO using the SMS PDU engine from oFono already, so you might switch over ;) To be honest it would be way simpler for you to write a MSM plugin for oFono since it is actually designed to support binary protocols like the Nokia Phonet in the first place. What do mean with that? If I look at fsogsmd (the vala implementation of the fso gsm daemon) it's even abstracted so I can easily implement a binary protocol as we found in the Palm Pre modem. If you wanna go for FSO, then by all means go for it. If they finally have a proper abstraction layer then all the better. With all the weird integration work we have done so far, we are pretty happy with our current design and have the flexibility we need and it worked out better for us than we ever dreamed it would. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Palm Pre modem plugin
Hi guys, The forth and worring is that a developer may be forced to use ofono, as the target device has some closed parts necessary for the os that does not work anymore if you remove ofono and use FSO. Please do not post these crazy conspiracy theories here. oFono is GPLed for exactly this reason. let me point this out once again. oFono is 100% GPL source code and this means that all plugins, drivers or extensions have to be released under GPL. This was our first and important item when creating oFono. We want a full open source telephony stack. No binary code running inside the daemon ever. Seems like everybody assumes that a GSM telephony system needs to have closed source components, but on the host CPU, we disagree with this and strongly enforce open source. We only draw the line with the UI elements like dialer etc. These are components that involve branding, customization and differentiation of products and since they talk via D-Bus they can be closed source. And to keep the complexity in the UI components minimal we handle all the details in the daemon. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Palm Pre modem plugin
Hi Morphis, I am currently working on the implementation of the basics I found out about the protocol. I split the whole code into two projects: msmcommd and libmsmcomm. msmcommd is a daemon which does all the link layer protocol handling which is required to speak probably with the modem. The library libmsmcomm is an easy utilitiy to format the different messages type and decode them on receive. The link layer part is nearly done. Establishment of a link should work and on the handling of sending and receiving data I am currently working. When libmsmcomm and msmcommd are done I have the plan to open from fsogsmd a network channel to msmcommd and send all my telephony commands to msmcommd whichs put them into link layer frames and sends them to the modem. I like the idea of libmsmcomm as a library, this means it may be easy in the future to make an oFono plugin linked against the library. I don't think having a separate daemon and another communication channel between what is eventually a dialer or sms app, and the modem is such a good idea (with the D-Bus socket, D-Bus daemon, FSO and serial driver already there) I do think that a native integration directly into the oFono source code as msmmodem might be better. However the second daemon solution is stupid and causes too much overhead. Also I did look into the QMI stuff a little bit and I think we might have to actually create a QMI subsystem in the Linux kernel like we have for Phonet since with USB the network interface and the management interface do share endpoints as far as I can tell so far. Whats QMI? Never heard about it. if you look through the Windows drivers and some of the .inf files, you find the QMI and most likely it means Qualcomm Management Interface. And there is also QMUX which is kind of a multiplexer similar to what Phonet does for the Nokia protocol. The QMI is similar to ISI from Nokia. Look through the MSM stuff from the Android kernel tree since they are also using some of these terms. And I guess all MSM modems are similar one way or the other. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: Patch on unsupported AT command
Hi Yang,
looks good so far, but ...
+static gboolean check_terminator(struct terminator_info *info, char
*line)
+{
+ if ((info-len == -1 !strcmp(line, info-terminator)) ||
+ (info-len 0 !strncmp(line, info-terminator,
info-len)))
+ return TRUE;
+ else
+ return FALSE;
+}
+
This is first of all violating the coding style with the indentation on
the second line of the if, but it is also way too complicated.
if (info-len == -1 !strcmp(line, info-terminator)
return TRUE;
if (info-len 0 !strncmp(line, info-terminator, ...))
return TRUE;
return FALSE;
Maybe it is too early in the morning to do code review, but this should
be doing the same, but be a lot simpler to read ;)
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Add mpty field to ofono_call
Hi Zhenhua, A field 'mpty' is added to ofono_call so that we can know whether the call belongs to multiparty call or not. According to 27.007 7.18, it is a defined return value of AT+CLCC. --- drivers/atmodem/atutil.c |4 +++- include/types.h |1 + 2 files changed, 4 insertions(+), 1 deletions(-) patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Palm Pre modem plugin
Hi Nicola, The forth and worring is that a developer may be forced to use ofono, as the target device has some closed parts necessary for the os that does not work anymore if you remove ofono and use FSO. Please do not post these crazy conspiracy theories here. oFono is GPLed for exactly this reason. May you elaborate about that? May not a close source project use oFono by calling it's DBus API? see my other email I wrote to explain it a little bit more. Yes, you can write an UI and keep it closed source. This is on purpose. However the D-Bus API is designed in a way that your UI is simple to begin with since we moved all the logic into the daemon. This is the big difference to the FSO GSM part. oFono does the heavy and complex work and all that is 100% GPL. Exactly how it should be. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Patch on unsupported AT command
Hi Denis, This is first of all violating the coding style with the indentation on the second line of the if, but it is also way too complicated. There is actually a reason for this. if (info-len == -1 !strcmp(line, info-terminator) return TRUE; This part checks for static terminators, like OK or BUSY or ERROR. We do whole string comparison here. if (info-len 0 !strncmp(line, info-terminator, ...)) return TRUE; This part checks for variable terminators. E.g. +CMS ERROR: XXX. These are well defined by the standard so we only do a prefix comparison for these. and your point is? I was just going by the pure algorithmic of the if statement to make it actually readable without getting a headache ;) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
First release with GPRS support
Hello everyone, I just tagged, pushed and uploaded 0.11 release of oFono. Besides the usual bug fixes in various areas, this comes now with GPRS support. If you happen to have a Ericsson MBM or Option HSO based modem, then you under lucky ones who can try GPRS support. To configure your Primary GPRS Context, you can use the create-context script from within the test directory: create-context apn You just have to give it the APN from your GPRS network settings. And then list-contexts will show you the configured context. A context needs to only configured once by SIM card. It will be stored on a per SIM card basis and restored after restart. To activate or deactivate the actual connect, activate-context and/or deactivate-context can be used. On success just call list-contexts again and you will see your IP settings. The integration of oFono GPRS support for ConnMan is work in progress and we hope to finish that pretty soon. So the next ConnMan release should be able to talk to oFono for data connections. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Added test script to enter pin to sim card.
Hi Ryan,
---
test/enter-pin.py | 28
1 files changed, 28 insertions(+), 0 deletions(-)
create mode 100755 test/enter-pin.py
don't forget to modify EXTRA_DIST of Makefile.am and please remove
the .py suffix. We don't do that with our test scripts.
diff --git a/test/enter-pin.py b/test/enter-pin.py
new file mode 100755
index 000..77de93a
--- /dev/null
+++ b/test/enter-pin.py
@@ -0,0 +1,28 @@
+#!/usr/bin/env python
+import dbus
+import sys
+
+bus = dbus.SystemBus()
+
+manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+ 'org.ofono.Manager')
+
+properties = manager.GetProperties()
+
+path = properties[Modems][0]
+
+if len(path)==0:
+print No modems found
+exit
+
+modem = dbus.Interface(bus.get_object('org.ofono', path),
+ 'org.ofono.SimManager')
+properties = modem.GetProperties()
+if properties.has_key('PinRequired') and properties['PinRequired'] ==
sys.argv[1] :
+modem.EnterPin(sys.argv[1], sys.argv[2])
+elif properties.has_key('PinRequired') and properties['PinRequired'] ==
'none' :
+print Pin not needed
+elif properties.has_key('PinRequired'):
+print Error: Pin type not supported (%s != %s) %
(sys.argv[1],properties['PinRequired'])
can we do something like if no argument has been provided, then check
for the current required PIN. So that you can get an easy way to tell
what is needed.
And then supplying the PIN type is pointless, isn't it. Just take the
first argument and set it as PIN. The type comes via PinRequired anyway.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: Patch on unsupported AT command
Hi Yang, Maybe it is too early in the morning to do code review, but this should be doing the same, but be a lot simpler to read ;) You're absolutely right. In this way, the code is more readable. Please review again. both patches have been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Huawei E220 + Data Connection Manager
Hi Mitsutaka,
I'm trying to implement for Huawei E220 using Data Connection
Manager. I understand GPRS data conneciton is working in progress. But
I don't know much connection sequence that.
So I'd like to tell me how should I approach. and I guess data
connection in the following. If I mistake, please point out.
1. Launch ofonod, plug-in the modem.
2. Enable modems for getting modem information, Model, Manufacture, Serial
and so on(test/enable-modem).
3. Create a context(org.ofono.PrimaryDataContext) according to
doc/dataconnectionmanager-api.txt
ex. Japanese carrier
% ./test/list-contexts
[ /huawei1 ]
[ /huawei1/primarycontext1 ]
Username = em
Name = Example
Settings = { }
Active = 0
AccessPointName = 1
Password = em
Type = internet
4. Activate it. but currently it can not be atacched
Traceback (most recent call last):
File ./test/activate-context, line 38, in module
context.SetProperty(Active, dbus.Boolean(1))
File /usr/lib/python2.6/site-packages/dbus/proxies.py, line 68, in
__call__
return self._proxy_method(*args, **keywords)
File /usr/lib/python2.6/site-packages/dbus/proxies.py, line 140, in
__call__
**keywords)
File /usr/lib/python2.6/site-packages/dbus/connection.py, line 622, in
call_blocking
message, timeout)
dbus.exceptions.DBusException: org.ofono.Error.NotAttached: GPRS is not
attached
5. Dialup using ATDT (this case is support only Tone)
ATDT*99***1#
CONNECT
6. ppp0 interface up
7. Authenticate EAP, PAP or CHAP using Username, Passowrd
8. Getting IP Address, Netmask, Gateway from APN(?)
so this is the interesting part. We don't wanna use the magic *99* magic
and actually use the proper GPRS AT command for connecting the context.
But then we have to talk PPP. That is currently work in progress as an
extension to GAtChat.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Hi Ismo, since we have the upstream GPRS support for MBM and HSO devices, it would be great to have this for Phonet, too. I tried to work on the patch, but without proper documentation it is impossible. So please adapt it to the current upstream and resend it. Yes, the GPRS ISI support has been delayed. Because the internal API between the Ofono GPRS atom and the modem driver was changed, the ISI driver won't work out of the box. I have been recently quite busy with other tasks, but I'll try to find the time to do the porting or preferably get Rémi to do it. :-) Since the ISI GPRS support was already demonstrated to be working, the actual work amount shouldn't be too big. I tried it by myself, but without the ISI specification, it is a pretty big effort. So we rely on you guys to do it. Do you have any estimate when this could be achieved. I am asking because this feature is also useful for desktop users wanting to attach their Nokia phone just for Internet access. Better than Dialup and PPP since they would get proper signal strength etc. with it. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: light test report for oFono v0.11
Hi Elvam Test Environment --- general PC (pre-installed FC11) - oFono v0.11, oFono v0.10 - phonesim package --- please update to Fedora 12 since I changed the actual build requirements for oFono to a more recent libudev package. In general not really needed when testing with phonesim, but still a good idea. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: GPRS support for Ofono
Hi Remi, I haven't had time to look at the context stuff yet. It's just worse. oFono is enforcing an integer ID on any primary context. This is again an idiosyncrasy of AT commands. With ISI modems, the context ID space is managed by the modem, so oFono does not get to choose. In theory, I could remap those, but that really would not make sense. there has to be a mapping on some level. At least one driver has to do some sort of mapping. If you have a better idea to handle it in the core daemon, please propose something. Please remember that non of us have the ISI specification and until you guys communicate these details with us, we go based on what public specifications are using. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Merry Christmas to everyone
Hello everyone, and Santa is coming to town and bringing the 0.15 release with him. Many thanks to all contributors during this year. We had 28 different contributors with all together 1464 commits in the public repository which goes back as far as April. The line count summary for a project that is fairly new is actually quite impressive: Language Files CodeComment Comment % Blank Total - - - - - - c 163 44816 4958 10.0% 13396 63170 python 29989 35 3.4%436 1460 automake 1251 0 0.0% 80331 autoconf 1132 1 0.8% 34167 shell 3 29 3 9.4% 8 40 - - - - - - Total 197 46217 4997 9.8% 13954 65168 So enjoy your vacation and a Happy New Year :) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] add netmask to hso gprs-context driver
Hi Martin,
diff --git a/drivers/hsomodem/gprs-context.c
b/drivers/hsomodem/gprs-context.c
index 0526fcc..721a017 100644
--- a/drivers/hsomodem/gprs-context.c
+++ b/drivers/hsomodem/gprs-context.c
@@ -47,6 +47,8 @@
#define AUTH_BUF_LENGTH OFONO_GPRS_MAX_USERNAME_LENGTH + \
OFONO_GPRS_MAX_PASSWORD_LENGTH + 128
+#define STATIC_IP_NETMASK 255.255.255.255
+
static const char *none_prefix[] = { NULL };
static const char *owandata_prefix[] = { _OWANDATA:, NULL };
@@ -274,7 +276,7 @@ static void owandata_cb(gboolean ok, GAtResult
*result, gpointer user_data)
ofono_info(IP: %s, Gateway: %s, ip, gateway);
ofono_info(DNS: %s, %s, dns1, dns2);
- CALLBACK_WITH_SUCCESS(gcd-up_cb, interface, TRUE, ip, NULL,
+ CALLBACK_WITH_SUCCESS(gcd-up_cb, interface, TRUE, ip,
STATIC_IP_NETMASK,
gateway, dns, gcd-cb_data);
your emails are all messed up. You are using multi-part MIME messages
with messed up context. I don't know what's wrong here, but this way I
am not able to apply any of your patches. Just use git send-email and
use linux.intel.com as SMTP server. That should do the right thing.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC 1/3] STE-plugin: Add vendor STE
Hi Sjur,
This patch add STE as vendor, and a few adjustments needed in atmodem
for ST-Ericsson modem.
---
drivers/atmodem/gprs.c | 15 +--
drivers/atmodem/vendor.h |5 +
plugins/modemconf.c |5 +
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index 76085d9..305f22f 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -17,6 +17,10 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA
*
+ * Copyright (C) 2010 ST-Ericsson AB.
+ * Author: Marit Henriksen, marit.xx.henrik...@stericsson.com.
+ * STE specific implementation.
+ *
*/
please don't do that. Add the proper copyright to the header if the
changes you are making are major. We track authorship via the GIT
itself.
#ifdef HAVE_CONFIG_H
@@ -38,6 +42,7 @@
#include gatresult.h
#include atmodem.h
+#include vendor.h
static const char *cgreg_prefix[] = { +CGREG:, NULL };
static const char *cgdcont_prefix[] = { +CGDCONT:, NULL };
@@ -45,6 +50,7 @@ static const char *none_prefix[] = { NULL };
struct gprs_data {
GAtChat *chat;
+ unsigned int vendor;
};
static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -216,7 +222,12 @@ static void at_cgreg_test_cb(gboolean ok, GAtResult
*result,
g_at_chat_send(gd-chat, cmd, none_prefix, NULL, NULL, NULL);
g_at_chat_send(gd-chat, AT+CGAUTO=0, none_prefix, NULL, NULL, NULL);
- g_at_chat_send(gd-chat, AT+CGEREP=2,1, none_prefix,
+
+ if (gd-vendor == OFONO_VENDOR_STE)
+ g_at_chat_send(gd-chat, AT+CGEREP=1,0, none_prefix,
+ gprs_initialized, gprs, NULL);
+ else
+ g_at_chat_send(gd-chat, AT+CGEREP=2,1, none_prefix,
gprs_initialized, gprs, NULL);
return;
Can you add some extra comment here explaining why this is needed.
Otherwise it looks like some black magic.
@@ -289,7 +300,7 @@ static int at_gprs_probe(struct ofono_gprs *gprs,
gd = g_new0(struct gprs_data, 1);
gd-chat = chat;
-
+ gd-vendor = vendor;
ofono_gprs_set_data(gprs, gd);
g_at_chat_send(chat, AT+CGDCONT=?, cgdcont_prefix,
diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index 8d9ed47..d7b5210 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -17,11 +17,16 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA
*
+ * Copyright (C) 2010 ST-Ericsson AB.
+ * Author: Marit Henriksen, marit.xx.henrik...@stericsson.com.
+ * STE specific implementation.
+ *
*/
To be honest, just adding a new entry in enum is not really a reason to
add a copyright statement here.
enum ofono_vendor {
OFONO_VENDOR_GENERIC = 0,
OFONO_VENDOR_CALYPSO,
+ OFONO_VENDOR_STE,
OFONO_VENDOR_QUALCOMM_MSM,
OFONO_VENDOR_OPTION_HSO,
};
diff --git a/plugins/modemconf.c b/plugins/modemconf.c
index c8c261f..41c7428 100644
--- a/plugins/modemconf.c
+++ b/plugins/modemconf.c
@@ -17,6 +17,10 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA
*
+ * Copyright (C) 2010 ST-Ericsson AB.
+ * Author: Marit Henriksen, marit.xx.henrik...@stericsson.com.
+ * STE specific implementation.
+ *
*/
#ifdef HAVE_CONFIG_H
@@ -128,6 +132,7 @@ static struct ofono_modem *create_modem(GKeyFile
*keyfile, const char *group)
set_address(modem, keyfile, group);
if (!g_strcmp0(driver, atgen) || !g_strcmp0(driver, g1) ||
+ !g_strcmp0(driver, ste) ||
!g_strcmp0(driver, calypso) ||
!g_strcmp0(driver, hfp) ||
!g_strcmp0(driver, palmpre))
I am failing to see the reason for this. I makes no sense to me. The
CAIF kernel code (as far as I understand it) exports AT command sockets
and they are configured differently. So this seems like a change without
any functionality.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC 3/3] STE-plugin: Adding STE plugin
Hi Sjur,
Added implementation for STE modem; STE modem driver, and STE specific
drivers for gprs, network registration and voice call.
This patch uses CAIF sockets. CAIF patch for net-next-2.6 will be
contributed on net...@vger.kernel.org soon.
can you please split these into smaller patches so they are easier to
review. I am thinking of something the like this; one per network
registration, one per voice call, one per GPRS.
You are making some core changes and we need to have a close look at
them and what the potential impact would be.
This is not a complete review, but I making some comments on obvious
things.
index 000..1d5d8db
--- /dev/null
+++ b/drivers/stemodem/gprs-context.c
@@ -0,0 +1,612 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2009 Intel Corporation. All rights reserved.
+ * Copyright (C) 2010 ST-Ericsson AB.
+ * Author: Marit Henriksen, marit.xx.henrik...@stericsson.com.
As mentioned before, we track authorship via GIT. So ensure that the GIT
commits are properly done.
+#include linux/types.h
+#include net/if.h
+#include sys/ioctl.h
+#include arpa/inet.h
+#include linux/caif/caif_socket.h
+#include linux/caif/if_caif.h
This is dangerous until the CAIF subsystem is actually merged and
present everywhere. Please consider adding an option to enable stemodem
driver (like we do for atmodem and isimodem).
It might make sense to have a local copy of the required structure and
constants to allow an easier complication. Of course this depends on
having CAIF at least in net-next-2.6 tree.
+/* TODO: should parse_xml function to be moved to e.g. atutil? */
First question. Why not use the XML parse that comes with GLib.
+static char *parse_xml(char * xml, char* tag)
+{
Please use consistent coding style. It is char *xml. And it is always
like this. No extra space after * or char*.
+char *begin;
+char *end;
+int len;
+char *res = NULL;
+char *start = (char *)g_malloc(strlen(tag)+3);
+char *stop = (char *)g_malloc(strlen(tag)+4);
No casting of malloc function. It is not needed. And extra spaces
between operation. Meaning malloc(strlen(tag) + 3).
This applies to all code.
+ if (create) {
+ if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
+ ofono_debug(Failed to create IP interface for CAIF);
+ goto error;
+ }
+
+ s = socket(PF_INET, SOCK_DGRAM, 0);
+ if (s 0) {
+ ofono_debug(Failed to create socket.);
+ goto error;
+ }
+
+ /* Set IP address */
+ memset(sin, 0, sizeof(struct sockaddr));
+ sin.sin_family = AF_INET;
+
+ if (inet_pton(AF_INET, ip, sin.sin_addr) = 0) {
+ ofono_debug(inet_pton failed, will not be
+ able to set the IP address);
+ goto error;
+ }
+ memcpy(ifr.ifr_addr, sin, sizeof(struct sockaddr));
+
+ if (ioctl(s, SIOCSIFADDR, ifr) 0) {
+ ofono_debug(Failed to set IP address for
+ interface: %s, ifr.ifr_name);
+ goto error;
+ }
oFono is never setting the IP address, netmask or any other
configuration option. The only thing that oFono does is bringing the
interface up. Systems like ConnMan do the IP configuration.
Please see the comments in the documentation on how we expose IP
interfaces. Check with ConnMan and how we configure them.
+ if (ioctl(s, SIOCSIFMTU, ifr) 0)
+ ofono_debug(Failed to set MTU for interface: %s,
+ ifr.ifr_name);
+ } else {
+ if (ioctl(s, SIOCGIFINDEX, ifr) == 0) {
+ if (ioctl(s, SIOCCAIFNETREMOVE, ifr) 0) {
+ ofono_debug(Failed to remove IP interface
+ for CAIF);
+ goto error;
+ }
+ } else {
+ ofono_debug(Did not find interface (%s) to remove,
+ interface);
+ goto error;
+ }
+ }
Having create and remove in the same function seems hard to read. Why
not have one for creating the interface and one for removing it. From
what I see so far, it is not much more code. And makes it a lot easier
to read and understand for other people.
+ g_at_result_iter_init(iter, result);
+ for (i = 0; i g_at_result_num_response_lines(result); i++) {
+ g_at_result_iter_next(iter, NULL);
+ res_string = strdup(g_at_result_iter_raw_line(iter));
+
+ if (strstr(res_string, ip_address)) {
+ ip = g_strdup(parse_xml(res_string,
+ ip_address));
+ } else if
RE: [RFC 3/3] STE-plugin: Adding STE plugin
Hi Sjur,
Thank you for your feedback. We hope to get new patch-set out tomorrow
with most of your comments fixed.
sounds great.
It might make sense to have a local copy of the required structure
and constants to allow an easier complication. Of course this depends
on having CAIF at least in net-next-2.6 tree.
OK, agree. I'll supply the CAIF specific patches next time.
Should we put the CAIF header files into ofono/include/caif?
Have a look at how we do it for the Phonet/ISI modem. Also we only need
a few constants (AF_CAIF, proto etc.) and mainly only the socket
structure for CAIF.
This should be easy to fix actually. More important is that we can
disable stemodem support via configure so people with an old kernel can
compile it. We are in a chicken and egg problem with the AF_CAIF
constant anyway. Until the CAIF subsystem is in net-next-2.6, the
actually number for it is not assigned.
+/* TODO: should parse_xml function to be moved to e.g. atutil? */
First question. Why not use the XML parse that comes with GLib.
OK. Is it Simple XML Subset Parser you refer to? If you have any pointer
to sample code using this XML parser we would appreciate it.
Yes, I am talking about that one. It is similar to expact and the only
example, I have is in the BlueZ source code. I think Google code search
will reveal more useful examples.
oFono is never setting the IP address, netmask or any other
configuration option. The only thing that oFono does is bringing the
interface up. Systems like ConnMan do the IP configuration.
Please see the comments in the documentation on how we expose IP
interfaces. Check with ConnMan and how we configure them.
OK, we're returning the relevant parameters from activate_primary so that
all PDP Context parameters including interface name,
ip-address, netmask, dns, etc are available in PrimaryDataContext.
And leave the interface created but not configured.
Does this sound ok?
Yes, sounds good. Please have a look at how we do it for Ericsson MBM
and Option HSO modems. We just send the D-Bus signal with all the
details and that is it.
I forgot to mention the reasoning behind. oFono is incapable of making
any kind of good decision when it comes to handling IP collision since
it only knows about its scope. Systems like ConnMan have the full
insight in whole networking to make proper decisions.
+ /* Need to change to gsm_permissive syntax in order to
+ * parse the response from EPPSD (xml) */
+ g_at_chat_set_syntax(gcd-chat, g_at_syntax_new_gsm_permissive());
Is this an issue with your modem firmware or an issue in the v1
parser.
If it is the modem firmware, then just use the permissive parser all
the time and switch to E0.
Setting permissive mode was done in order to be able to parse
the XML response from EPPSD (Propriatery Activate PDP context).
But we can try if we can run with this mode default if you think
this is better.
Can you post an example (manually via cu or minicom and with ATE1) on
how this looks like. My question here is if you are actually following
the strict v1 syntax. If you don't then that is basically a bug on the
modem side. I don't even know if v1 has any kind of capabilities to
handle XML properly in the first place.
The point here is if you can not use v1 parser, then just use permissive
from the beginning and avoid switching at runtime. The capability of
switching at runtime mainly only exists for testing purposes. With the
permissive parser you just have to ensure ATE0 inside the modem plugin.
See the other users for an example.
+static void cgev_notify(GAtResult *result, gpointer user_data) {
+ struct ofono_gprs_context *gc = user_data;
+ struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
+ GAtResultIter iter; + const char *event;
+
+ dump_response(cgev_notify, TRUE, result);
+
+ g_at_result_iter_init(iter, result);
+
+ if (!g_at_result_iter_next(iter, +CGEV:))
+ return;
+
+ if (!g_at_result_iter_next_unquoted_string(iter, event))
+ return; +
+ if (g_str_has_prefix(event, NW REACT ) ||
+ g_str_has_prefix(event, NW DEACT ) ||
+ g_str_has_prefix(event, ME DEACT )) {
+ /* Ask what primary contexts are active now */
+
+ g_at_chat_send(gcd-chat, AT+CGACT?, cgact_prefix,
+ ste_cgact_read_cb, gc, NULL);
+
+ return;
+ }
+}
The return statement in the if clause is kinda pointless. Seems like
either your code tried to be more complex or you are missing
something.
Sure we can fix this, but this is actually just copied from gprs_context in
atmodem.
BTW, the iter_init seems to be missing in atmodem's implementation, this is
probably
a bug in atmodem.
Sounds like a bug in atmodem then. Can you point me to the lines where
you are seeing this. I will fix it if needed.
And btw. the { for
Re: [RFC v2 0/7] STE-plugin: Second RFC on STE patch-set
Hi Sjur, This is the second RFC on the STE driver.Most review comments should be included, except for using glib for XML parsing and changes to voice-call termination. We'll have to get back to this later. we are getting to closer to something that can be merged as a first draft and then later extended. Sjur Brandeland (7): STE-plugin: Add --disable-stemodem to makefiles. STE-plugin: Add vendor STE and quirks in atmodem. STE-plugin: STE modem driver definition STE-plugin: Add STE plugin STE-plugin: Voicecall driver. STE-plugin: Add CAIF header files in gcaif. STE-plugin: GPRS driver. So the order of these patches is kinda messed up. Think about it like this; only adding patch 1 makes the compilation and hence git bisect break. That should be the last patch. Also you are adding CAIF headers after you already rely on them in a previous patch. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC v2 3/7] STE-plugin: STE modem driver definition
Hi Sjur,
Updates after review: Removed overriding of functions.
---
snip
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include glib.h
+#include gatchat.h
What are these two includes for?
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include ofono/plugin.h
+#include ofono/types.h
Why do you need the ofono/types.h include. Seems like some leftovers.
+#include stemodem.h
+
+static int stemodem_init(void)
+{
+ ste_voicecall_init();
+ ste_gprs_context_init();
+
+ return 0;
+}
As I explained in the introductory reply. You have to keep the order
properly. So this patch either has to come last or the modem init needs
to be empty. You don't have the voicecall or GPRS init function merged
at this point.
If you wanna merge the stemodem.c and the configure option at some
point, I am perfectly fine with that. Just make it an empty stub here so
we are not breaking git bisect.
Together with the other two patches, I would apply that right away.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC v2 4/7] STE-plugin: Add STE plugin
Hi Sjur,
Added check on return value of socket() and connect() calls.
Use gsm_permissive syntax all the time.
Added ATE0 in initialization.
snip
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include errno.h
+#include stdlib.h
+
+#include glib.h
+#include gatchat.h
+#include gattty.h
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include ofono/plugin.h
+#include ofono/log.h
+#include ofono/modem.h
+#include ofono/call-barring.h
+#include ofono/call-forwarding.h
+#include ofono/call-meter.h
+#include ofono/call-settings.h
+#include ofono/devinfo.h
+#include ofono/message-waiting.h
+#include ofono/netreg.h
+#include ofono/phonebook.h
+#include ofono/sim.h
+#include ofono/sms.h
+#include ofono/ssn.h
+#include ofono/ussd.h
+#include ofono/call-volume.h
+#include ofono/voicecall.h
+#include drivers/atmodem/vendor.h
+#include ofono/gprs.h
+#include unistd.h
+#include ofono/gprs-context.h
please, please only include the atom header files you really use.
+/* Use the local CAIF header files until is included in linux kernel */
+#ifdef PF_CAIF
+#include linux/caif/caif_socket.h
+#include linux/caif/if_caif.h
+#else
+#include gcaif/caif_socket.h
+#include gcaif/if_caif.h
+#endif
This is not gonna work. To make this work you would need a GLibc that
has the PF_CAIF defined.
My current proposal to fix this would be to include caif_socket.h and
if_caif.h in the drivers/stemodem/ directory and just use them directly.
The gcaif/ directory is the wrong approach here. We have the g* prefixed
subdirectories for libraries that provide GLib mainloop integration. You
are not doing that, you just need to extra headers. So put them side by
side with the modem driver.
We can fix the upstream usage of CAIF and their header files later then
when that has been accepted into net-next-2.6 tree.
+ int fd, err;
+ struct sockaddr_caif addr = {
+ .family = AF_CAIF,
+ .u.at.type = CAIF_ATTYPE_PLAIN
+ };
I would prefer if you do this like this:
memset(addr, 0, sizeof(addr));
addr.family = AF_CAIF;
addr.u.at.type = CAIF_ATTYPE_PLAIN;
And just before calling connect.
Also the more important question here is if you have two CAIF modems in
the system. How do you distinguish between them?
+
+ DBG(%p, modem);
+
+ /* Create a CAIF socket for AT Service */
+ fd = socket(AF_CAIF, SOCK_SEQPACKET, CAIFPROTO_AT);
+
+ if (fd 0) {
Skip that extra empty line here.
+ ofono_debug(Failed to create CAIF socket for AT);
+ return -EIO;
+ }
+ /* Connect to the AT Service at the modem */
+ err = connect(fd, (struct sockaddr *) addr, sizeof(addr));
+
+ if (err 0) {
Same here.
+ close(fd);
+ return err;
+ }
+ channel = g_io_channel_unix_new(fd);
+
+ if (!channel) {
And here.
+static void ste_pre_sim(struct ofono_modem *modem)
+{
+ struct ste_data *data = ofono_modem_get_data(modem);
+ DBG(%p, modem);
+
+ ofono_devinfo_create(modem, 0, atmodem, data-chat);
+ ofono_sim_create(modem, 0, atmodem, data-chat);
+ ofono_voicecall_create(modem, 0, stemodem, data-chat);
+}
It will not break the build, but the functionality for the stemodem
voiceall atom is not merged yet. So leave that out.
+static void ste_post_sim(struct ofono_modem *modem)
+{
+ struct ste_data *data = ofono_modem_get_data(modem);
+ struct ofono_message_waiting *mw;
+ struct ofono_gprs *gprs;
+ struct ofono_gprs_context *gc;
+
+ DBG(%p, modem);
+ ofono_ussd_create(modem, 0, atmodem, data-chat);
+ ofono_call_forwarding_create(modem, 0, atmodem, data-chat);
+ ofono_call_settings_create(modem, 0, atmodem, data-chat);
+ ofono_netreg_create(modem, OFONO_VENDOR_STE, atmodem, data-chat);
+ ofono_call_meter_create(modem, 0, atmodem, data-chat);
+ ofono_call_barring_create(modem, 0, atmodem, data-chat);
+ ofono_ssn_create(modem, 0, atmodem, data-chat);
+ ofono_sms_create(modem, 0, atmodem, data-chat);
+ ofono_phonebook_create(modem, 0, atmodem, data-chat);
+ ofono_call_volume_create(modem, 0, atmodem, data-chat);
+
+ gprs = ofono_gprs_create(modem,
+ OFONO_VENDOR_STE, atmodem, data-chat);
+ gc = ofono_gprs_context_create(modem, 0, stemodem, data-chat);
Same applies here. Please don't do this until all the dependencies are
merged.
You can just add these extra atom via the patch introducing the support
for it. That makes the git log way cleaner and we can follow the changes
in a lot simpler way at some later time.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC v2 5/7] STE-plugin: Voice-call driver.
Hi Sjur, Updates after review: Use at_util_call_compare. So far, we have only done the trivial changes to voice call driver. We agree on moving more of the state logic to core, but this is complex and we need more time to discuss and look into this, particularly regarding call release. so this needs a bit more detailed review from Denis once he is back. I propose that you update all the patches except voice call and GPRS and make a separate series of patches from them. Then we can merge them and discuss voicecall and GPRS after that. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC v2 6/7] STE-plugin: Add CAIF header files in gcaif.
Hi Sjur, Adding CAIF header files. These should be removed when CAIF is included Linux kernel. please put them into drivers/stemodem/ instead as I explained in the other email. And make sure they are before its actual usage in the patch series. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC v2 7/7] STE-plugin: GPRS driver.
Hi Sjur, Updates after review: Removed setting of IP-address, netmask etc. when creating the IP interface. Split caif_if_create_remove() into , caif_if_create() and caif_if_remove(). Declared interface as array in conn_info struct, and removed memory allocation. Removed switching of syntax runtime (from strict to permissive and back again). Removed return statement from cgev_notify() Remaining issues: Use glib xml parser. I am not comfortable with merging this until we have a version that uses the GLib builtin XML parser. As mentioned before, lets get the core patches merged and then we work on voicecall and GPRS in separate patch reviews. That makes it simpler. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Add HFP support through BlueZ
Hi Gustavo, It uses BlueZ through to get HFP working following the org.bluez.HandsfreeGateway and org.bluez.HandsfreeAgent from the BlueZ D-Bus API. You need the HFP suport into BlueZ and the new dbus 1.3 with fd-passing support. Many thanks to Zhenhua Zhang zhenhua.zh...@intel.com for its prototype on this code. this patch has been applied to the oFono tree. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Add STE voice call support.
Hi Sjur,
---
Makefile.am |1 +
drivers/stemodem/stemodem.c |2 +
drivers/stemodem/stemodem.h |3 +
drivers/stemodem/voicecall.c | 596
++
plugins/ste.c|2 +-
5 files changed, 603 insertions(+), 1 deletions(-)
create mode 100644 drivers/stemodem/voicecall.c
diff --git a/Makefile.am b/Makefile.am
index ac13d73..ecd2660 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -159,6 +159,7 @@ builtin_sources += drivers/atmodem/atutil.h \
drivers/stemodem/stemodem.h \
drivers/stemodem/stemodem.c \
drivers/stemodem/gprs-context.c \
+ drivers/stemodem/voicecall.c \
drivers/stemodem/caif_socket.h \
drivers/stemodem/if_caif.h
diff --git a/drivers/stemodem/stemodem.c b/drivers/stemodem/stemodem.c
index 53207db..9184a42 100644
--- a/drivers/stemodem/stemodem.c
+++ b/drivers/stemodem/stemodem.c
@@ -37,6 +37,7 @@
static int stemodem_init(void)
{
ste_gprs_context_init();
+ ste_voicecall_init();
return 0;
}
@@ -44,6 +45,7 @@ static int stemodem_init(void)
static void stemodem_exit(void)
{
ste_gprs_context_exit();
+ ste_voicecall_exit();
}
OFONO_PLUGIN_DEFINE(stemodem, STE modem driver, VERSION,
diff --git a/drivers/stemodem/stemodem.h b/drivers/stemodem/stemodem.h
index e55a2c3..e7c6934 100644
--- a/drivers/stemodem/stemodem.h
+++ b/drivers/stemodem/stemodem.h
@@ -25,3 +25,6 @@
extern void ste_gprs_context_init();
extern void ste_gprs_context_exit();
+extern void ste_voicecall_init();
+extern void ste_voicecall_exit();
+
just a small style issue. Can you put voicecall before GPRS in the init
routines and the Makefile.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: RFC: API for Neighbouring Cell Info
Hi Jussi, Honestly I don't like either approach, the Agent pattern would be a much better fit here. This would allow us to specify the polling/update interval and stop neighbor cell updates when no one is interested in them. In my experience, the positioning guys don't need periodic updates at all. The data needs to be fetched on demand. Like whenever the user starts up a location-aware application. I'm willing to bet this is a chicken-egg problem: Why design software that uses periodic location updates if they aren't available? Only exposing plain polling is fine if it makes sense in a powersaving (and api simplicity) sense: a version of periodic updating can always be implemented at the position service (geoclue) level if it seems useful. there is no problem in polling the hardware if we have to. However if that is the case, then we obviously should only poll when we have a user/client asking for the information. And we should poll in an interval that the user/client needs this information. As Denis mentioned, if we have a D-Bus API for retrieving these information then we need to able to handle multiple users in a smart way without wasting system resources or blocking each other. I consider this similar to usage statistics. And in ConnMan I went for an agent concept to solve this. The similar approach seems to fit here perfectly if we wanna expose these information over D-Bus. That is the only way, we have control over clients. If nothing has changed there is not point in waking up more processes and let the determine noting has changed to only go back to sleep afterwards. If these clients actually have UI components that redraw, it is even worse. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Fix: Check if interface exists before creating it.
Hi Sjur,
diff --git a/drivers/stemodem/gprs-context.c b/drivers/stemodem/gprs-context.c
index c178fb6..79e697b 100644
--- a/drivers/stemodem/gprs-context.c
+++ b/drivers/stemodem/gprs-context.c
@@ -187,9 +187,11 @@ static gboolean caif_if_create(const char *interface,
unsigned int connid)
return FALSE;
}
- if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
- ofono_debug(Failed to create IP interface for CAIF);
- return FALSE;
+ if (ioctl(s, SIOCGIFINDEX, ifr) != 0) {
+ if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
+ ofono_debug(Failed to create IP interface for CAIF);
+ return FALSE;
+ }
}
return TRUE;
just doing it like this would be better:
/* Check if interface exists */
if (ioctl(s, SIOCGIFINDEX, ifr) == 0)
return TRUE;
if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
...
return FALSE;
}
return TRUE;
We are not a big fan complicated if clauses if they can be avoid with a
simple goto or just a return.
Also I think we need to check errno value here. Since potentially the
ioctl can fail for other reasons. And maybe extending CAIF with a proper
way of checking for an existing interface might be better.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] Fix: Username and Password must be set after context is created.
Hi Sjur,
diff --git a/drivers/stemodem/gprs-context.c b/drivers/stemodem/gprs-context.c
index 6743aba..c178fb6 100644
--- a/drivers/stemodem/gprs-context.c
+++ b/drivers/stemodem/gprs-context.c
@@ -416,7 +416,7 @@ static void ste_gprs_activate_primary(struct
ofono_gprs_context *gc,
{
struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
struct cb_data *cbd = cb_data_new(cb, data);
- char buf[AUTH_BUF_LENGTH];
+ char buf[2*AUTH_BUF_LENGTH];
int len;
if (!cbd)
@@ -425,19 +425,17 @@ static void ste_gprs_activate_primary(struct
ofono_gprs_context *gc,
gcd-active_context = ctx-cid;
cbd-user = gc;
- /* Set username and password */
- sprintf(buf, AT*EIAAUW=%d,1,\%s\,\%s\, ctx-cid,
- ctx-username, ctx-password);
-
- if (g_at_chat_send(gcd-chat, buf, none_prefix, NULL, NULL, NULL) == 0)
- goto error;
-
len = sprintf(buf, AT+CGDCONT=%u,\IP\, ctx-cid);
if (ctx-apn)
snprintf(buf + len, sizeof(buf) - len - 3, ,\%s\,
ctx-apn);
+ /* Set username and password. Must be done after context creation */
+ len = strlen(buf);
+ sprintf(buf+len, ;*EIAAUW=%d,1,\%s\,\%s\, ctx-cid,
+ ctx-username, ctx-password);
+
if (g_at_chat_send(gcd-chat, buf, none_prefix,
ste_cgdcont_cb, cbd, g_free) 0)
return;
this looks pretty much complicated and I prefer we don't use this crazy
concat of AT commands. Also it violates the coding style.
There is no problem to just use g_at_chat_send twice since it will queue
the commands for you properly. However if this really depends on CGDCONT
succeeding, then we better do it in the callback.
And we might wanna check if MBM cards behave similar and ensure that STE
and MBM cards use a similar code flow.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/2] Fix: Username and Password must be set after context is created.
Hi Sjur, len = sprintf(buf, AT+CGDCONT=%u,\IP\, ctx-cid); if (ctx-apn) snprintf(buf + len, sizeof(buf) - len - 3, ,\%s\, ctx-apn); + /* Set username and password. Must be done after context creation */ + len = strlen(buf); + sprintf(buf+len, ;*EIAAUW=%d,1,\%s\,\%s\, ctx-cid, + ctx-username, ctx-password); + if (g_at_chat_send(gcd-chat, buf, none_prefix, ste_cgdcont_cb, cbd, g_free) 0) return; this looks pretty much complicated and I prefer we don't use this crazy concat of AT commands. Also it violates the coding style. There is no problem to just use g_at_chat_send twice since it will queue the commands for you properly. However if this really depends on CGDCONT succeeding, then we better do it in the callback. We originally did send EIAAUW in the callback, but Denis changed this because MBM did it EIAAUW before CGDCONT. I know that Denis changed it and it makes sense to keep this in sync. We just don't know about these details. And we have to put comments in the source code to remind us. And to be honest, I am not sure if anybody really tested it. I only know of one public network that requires username/password for their APN. I need to get a SIM card from them once I am back in Germany. And we might wanna check if MBM cards behave similar and ensure that STE and MBM cards use a similar code flow. When testing this EIAAUW fails if there are no prior PDP context on the modem. Most likely this is the case with the MBM module as well, but I have not tested this. Then we have to do that in the callback in an extra step. I don't really like it, but seems the way to go. Please send a patch that also changes this for MBM devices. We really wanna keep these in sync. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/2] Fix: Check if interface exists before creating it.
Hi Sjur,
diff --git a/drivers/stemodem/gprs-context.c
b/drivers/stemodem/gprs-context.c index c178fb6..79e697b 100644
--- a/drivers/stemodem/gprs-context.c
+++ b/drivers/stemodem/gprs-context.c
@@ -187,9 +187,11 @@ static gboolean caif_if_create(const char
*interface, unsigned int connid)return FALSE; }
- if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
- ofono_debug(Failed to create IP interface for CAIF);
- return FALSE;
+ if (ioctl(s, SIOCGIFINDEX, ifr) != 0) {
+ if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
+ ofono_debug(Failed to create IP interface for CAIF);
+ return FALSE; + }
}
return TRUE;
just doing it like this would be better:
/* Check if interface exists */
if (ioctl(s, SIOCGIFINDEX, ifr) == 0)
return TRUE;
if (ioctl(s, SIOCCAIFNETNEW, ifr) 0) {
...
return FALSE;
}
return TRUE;
We are not a big fan complicated if clauses if they can be avoid with
a simple goto or just a return.
Also I think we need to check errno value here. Since potentially the
ioctl can fail for other reasons. And maybe extending CAIF with a
proper way of checking for an existing interface might be better.
Do you have anything particular in mind here?
Maybe CAIF could simply return EEXIST from SIOCCAIFNETNEW if the interface
already exists? In this case we could return TRUE anyway.
if it isn't that doing already, then it should be doing it. And yes,
just returning TRUE in that case seems fine to me.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] Fix: Username and Password must be set after context is created.
Hi Denis, And we might wanna check if MBM cards behave similar and ensure that STE and MBM cards use a similar code flow. When testing this EIAAUW fails if there are no prior PDP context on the modem. Most likely this is the case with the MBM module as well, but I have not tested this. Then we have to do that in the callback in an extra step. I don't really like it, but seems the way to go. Please send a patch that also changes this for MBM devices. We really wanna keep these in sync. Actually the easiest way to do this is simply queue the EIAAUW after the CGDCONT in activate_context. If CGDCONT fails then EIAAUW won't have any effect anyway (but will get executed nevertheless). This is better than worrying about strduping username/password and freeing it in all possible code paths. sounds good enough to me. Just wanna have STE and MBM in sync here. And of course an extra comment that explain why we do it. Sjur, care to send a patch? Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] Add radio settings atom and driver API
Hi Aki, +Properties string Mode [read-write] + + The current radio access selection mode, also known + as network preference. + + The possible values are: + dual Radio access selection is done +automatically, using either 2G +or 3G, depending on reception. + 2g Only GSM used for radio access. + 3g Only UTRAN used for radio access. + unknown Mode currently unknown. how would we be dealing with LTE here. The word dual is kinda tricky in that sense. Also what about chips like Qualcomm GOBI that can change their firmware and start operating in CDMA. We do wanna extend oFono to support CDMA at some point. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] Add radio settings atom and driver API
Hi Aki, +Propertiesstring Mode [read-write] + +The current radio access selection mode, also known +as network preference. + +The possible values are: +dualRadio access selection is done +automatically, using either 2G +or 3G, depending on reception. +2gOnly GSM used for radio access. +3gOnly UTRAN used for radio access. +unknown Mode currently unknown. how would we be dealing with LTE here. The word dual is kinda tricky in that sense. Also what about chips like Qualcomm GOBI that can change their firmware and start operating in CDMA. We do wanna extend oFono to support CDMA at some point. Yeah, perhaps the property should be called TechnologyPreference with values any, 2G, 3G, 4G. For now at least. so what about the magic CDMA + UMTS combo chips. We basically have to reload the firmware with these. Would be nice to figure out on how that can be done. Not that we know much about how Qualcomm intends to use that in a phone case scenario, but would be nice to at least think about it a bit. Also I wanted you to ask about the data roaming. Does the Nokia modem has an option to NOT data roam that we can set. The problem with doing it in the host is that a bit of data goes through if you start roaming and have a PDP context active. So it costs the user. At least some reports indicate this. Does your modem forces a detach or does it just happily data roam and just give us a cell update? The AT command specification obviously didn't care about this at all :( Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] Add radio settings atom and driver API
Hi Aki, Yeah, perhaps the property should be called TechnologyPreference with values any, 2G, 3G, 4G. For now at least. so what about the magic CDMA + UMTS combo chips. We basically have to reload the firmware with these. Would be nice to figure out on how that can be done. Not that we know much about how Qualcomm intends to use that in a phone case scenario, but would be nice to at least think about it a bit. I'm not at all sure toggling between CDMA and UMTS is going to work smoothly enough for it to be done at this interface level. It sounds more like disabling one modem and enabling another. And I'm sure there are other areas in the API that are going to need tweaking for CDMA. I would rather cross that bridge when we come to it. fair enough from my point of view, but lets keep that in mind. The hardware for that is out there. Also I wanted you to ask about the data roaming. Does the Nokia modem has an option to NOT data roam that we can set. The problem with doing it in the host is that a bit of data goes through if you start roaming and have a PDP context active. So it costs the user. At least some reports indicate this. Does your modem forces a detach or does it just happily data roam and just give us a cell update? You mean PDP contexts survive a network change? I don't think they do. Sounds like a bug in the application. The Nokia modems have a setting to automatically attach, but that doesn't yet move any data. That is what T-Mobile promises me when crossing borders into Netherlands or Austria with my German SIM card. It should have smooth data roaming. To be honest, I have never really tried it. Either I was on a plane anymore or too cheap to pay for data roaming ;) Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/1] install data file
Hi Martin, Makefile.am |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile.am b/Makefile.am index fcee5e6..007d5b9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -45,7 +45,7 @@ INCLUDES = -I$(top_srcdir)/src -I$(top_builddir)/src CLEANFILES = src/control.moc $(nodist_src_phonesim_SOURCES) -EXTRA_DIST = src/controlbase.ui src/default.xml src/GSMSpecification.xml +dist_pkgdata_DATA = src/controlbase.ui src/moblin.xml src/GSMSpecification.xml this looks wrong. You should not be installing src/controlbase.ui and don't bother installing GSMSpecification.xml. I just put that into the source code for reference. Also if you wanna install default.xml, then it needs to be installed in /etc/phonesim/ and not just somewhere. Please run a make install DESTDIR=$PWD/x as normal user from the build directory of phonesim. Then find x will show you the directory layout of the installation. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 1/6] Add PPP protocol support with HDLC framing
Hi Denis, +struct ppp_link * g_at_ppp_new(GIOChannel *modem); +void g_at_ppp_open(struct ppp_link *link); +void g_at_ppp_set_connect_function(struct ppp_link *link, + GAtPPPConnectFunc callback, gpointer user_data); +void g_at_ppp_set_disconnect_function(struct ppp_link *link, + GAtPPPDisconnectFunc callback, + gpointer user_data); +void g_at_ppp_shutdown(struct ppp_link *link); +void g_at_ppp_ref(struct ppp_link *link); +void g_at_ppp_unref(struct ppp_link *link); Almost forgot, let us not use struct ppp_link here, but instead use GAtPpp *ppp as the parameter. Again, mostly for consistency with how glib / gatchat does this. I am actually okay with GAtPPP since it looks much nicer. True camel case is making it harder to read. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 6/6] Allow gsmdial to use gatchat ppp support
Hi Kristen,
Add option to use PPP to gsmdial.
Index: ofono/gatchat/gsmdial.c
===
--- ofono.orig/gatchat/gsmdial.c 2010-03-10 16:58:09.773080389 -0800
+++ ofono/gatchat/gsmdial.c 2010-03-10 17:06:45.071975512 -0800
@@ -33,6 +33,9 @@
#include glib.h
#include gatchat.h
#include gattty.h
+#include arpa/inet.h
+#include net/if.h
+#include gatppp.h
please put system includes before glib or internal includes.
static const char *none_prefix[] = { NULL };
static const char *cgreg_prefix[] = { +CGREG:, NULL };
@@ -45,10 +48,14 @@
static gchar *option_apn = NULL;
static gint option_offmode = 0;
static gboolean option_legacy = FALSE;
+static gboolean option_ppp = FALSE;
+static gchar *option_username = NULL;
+static gchar *option_passwd = NULL;
Just use option_password here. Shortening is not really useful in this
case.
static GAtChat *control;
static GAtChat *modem;
static GMainLoop *event_loop;
+static struct ppp_link *ppp;
enum state {
STATE_NONE = 0,
@@ -220,6 +227,70 @@
g_at_chat_send(modem, buf, none_prefix, NULL, NULL, NULL);
}
+static void print_ip_address(guint32 ip_addr)
+{
+ struct in_addr addr;
+ addr.s_addr = ip_addr;
+ g_print(%s\n, inet_ntoa(addr));
It is just fine to use printf() like everybody else. The g_print
function is pretty much stupid idea from GLib. Same as gchar should not
be used either.
The only case where g_print makes sense is in GLib based unit tests.
And personally I would do
static void print_ip_address(const char *label, guint32 addr)
{
}
That way you can later on just call one function and don't have to
manually print the label first.
+static void ppp_connect(struct ppp_link *link, gint success, guint32 ip_addr,
+ guint32 dns1, guint32 dns2, gpointer user_data)
+{
+ if (success == PPP_CONNECT_SUCCESS) {
+ /* print out the negotiated address and dns server */
+ g_print(Negotiated IP Address: );
+ print_ip_address(ip_addr);
+
+ g_print(Primary DNS Server: );
+ print_ip_address(dns1);
+
+ g_print(Secondary DNS Server: );
+ print_ip_address(dns2);
+ } else {
+ g_print(Failed to create PPP interface!\n);
+ }
+}
So personally I prefer the style more like this:
if (success != PPP_CONNECT_SUCCESS) {
printf(... err msg ...);
return;
}
printf(... success ...)
It is a lot better to get the error cases out of your way first and then
just focus on the success case. Instead of handling success and then
btw. we have to take of the error case.
+static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ GIOChannel *channel;
+
+ if (!ok) {
+ g_print(Unable to define context\n);
+ exit(1);
+ }
+
+ /* get the data IO channel */
+ channel = g_at_chat_get_channel(modem);
+ channel = g_io_channel_ref(channel);
What is this reference for? The g_at_ppp_new() should take a reference
on that channel anyway.
+ /* shutdown gatchat */
+ g_at_chat_unref(control);
+ g_at_chat_unref(modem);
If you try to protect these, then I prefer that you call
g_at_chat_shutdown() here first and not directly unref them.
We should be able to keep the chat object around even if PPP is running
now.
+ /* open ppp */
+ ppp = g_at_ppp_new(channel);
+ if (ppp) {
+ g_print(Setting PPP Credentials to %s, %s\n, option_username,
+ option_passwd);
+ g_at_ppp_set_credentials(ppp, option_username,
+ option_passwd);
+
+ /* set connect and disconnect callbacks */
+ g_at_ppp_set_connect_function(ppp, ppp_connect, NULL);
+ g_at_ppp_set_disconnect_function(ppp, ppp_disconnect, NULL);
+
+ /* open the ppp connection */
+ g_at_ppp_open(ppp);
+ }
+}
This should be again done like this:
if (!ppp) {
printf(... err msg ..)
return;
}
g_at_ppp_set_credentials
static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
{
char buf[64];
@@ -231,8 +302,14 @@
if (option_legacy == TRUE) {
sprintf(buf, ATD*99***%u#, option_cid);
- g_at_chat_send(modem, buf, none_prefix,
- NULL, NULL, NULL);
+ if (option_ppp == TRUE) {
+ g_print(Option PPP enabled\n);
+ g_at_chat_send(modem, buf, none_prefix,
+ connect_cb, NULL, NULL);
+ }
+ else
+ g_at_chat_send(modem, buf,
Re: [PATCH] Add support of Huawei EM770 modem
Hi Yang, Comparing with general Huawei modem, EM770 is a full feature modem that supports voicecall, phonebook, call forwarding, call barring, etc. --- Makefile.am|3 + plugins/huawei-em770.c | 226 plugins/ofono.rules|1 + plugins/udev.c | 27 ++ 4 files changed, 257 insertions(+), 0 deletions(-) create mode 100644 plugins/huawei-em770.c for simplicity just let call it em770.c and everything where you have huawei_em770 just use em770. We do similar things for G1 etc. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Add support of Huawei EM770 modem
Hi Yang, Comparing with general Huawei modem, EM770 is a full feature modem that supports voicecall, phonebook, call forwarding, call barring, etc. --- Makefile.am |3 + plugins/em770.c | 226 +++ plugins/ofono.rules |1 + plugins/udev.c | 27 ++ 4 files changed, 257 insertions(+), 0 deletions(-) create mode 100644 plugins/em770.c patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/4] Add parser for file list objects
Hi Yang,
---
src/stkutil.c | 42 ++
src/stkutil.h |6 ++
2 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/src/stkutil.c b/src/stkutil.c
index ceba2d5..9f3bc0b 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -406,6 +406,46 @@ static gboolean parse_dataobj_tone(struct
comprehension_tlv_iter *iter,
return TRUE;
}
+/* Defined in TS 102.223 Section 8.18 */
+static gboolean parse_dataobj_file_list(struct comprehension_tlv_iter *iter,
+ void *user)
+{
+ GSList **fl = user;
+ const unsigned char *data;
+ unsigned int len;
+ unsigned int i;
+ unsigned int start = 1;
+ struct stk_file *sf;
+
+ if (comprehension_tlv_iter_get_tag(iter) !=
+ STK_DATA_OBJECT_TYPE_FILE_LIST)
+ return FALSE;
+
+ len = comprehension_tlv_iter_get_length(iter);
+ if (len 5)
+ return FALSE;
+
+ data = comprehension_tlv_iter_get_data(iter);
+
+ if (data[start] != 0x3f)
+ return FALSE;
+
+ for (i = start + 4; i = len; i += 2) {
+ if ((data[i] == 0x3f) || (i == len)) {
+ sf = g_new0(struct stk_file, 1);
+ sf-file = g_malloc(i-start);
+ memcpy(sf-file, data+start, i-start);
+ sf-len = i - start;
+ *fl = g_slist_prepend(*fl, sf);
+ start = i;
+ }
doing a simple continue statement is way better.
if (data[i] != 0x3f i != len)
continue;
Also there are not extra braces needed around expression in an if
statement. And i-start would have to be i - start.
When using g_new0 and g_malloc you result in exit on OOM. We try to
avoid that whenever possible. So I prefer using g_try_new0 and
g_try_malloc except there is a reason to just accept exit on OOM.
Personally the whole counting looks pretty complicated. I think it needs
either some documentation or made simpler.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 4/4] Add parser for location information objects
Hi Yang,
src/simutil.c |2 +-
src/simutil.h |1 +
src/stkutil.c | 40 +---
src/stkutil.h |9 +
4 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/src/simutil.c b/src/simutil.c
index d9383b7..65ffa36 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -538,7 +538,7 @@ static char *sim_network_name_parse(const unsigned char
*buffer, int length,
return ret;
}
-static void parse_mcc_mnc(const guint8 *bcd, char *mcc, char *mnc)
+void parse_mcc_mnc(const guint8 *bcd, char *mcc, char *mnc)
{
static const char digit_lut[] = 0123456789*#abd\0;
guint8 digit;
diff --git a/src/simutil.h b/src/simutil.h
index 043c21f..09964a8 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -181,6 +181,7 @@ const struct sim_eons_operator_info
*sim_eons_lookup(struct sim_eons *eons,
const char *mnc);
void sim_eons_free(struct sim_eons *eons);
+void parse_mcc_mnc(const guint8 *bcd, char *mcc, char *mnc);
struct sim_spdi *sim_spdi_new(const guint8 *tlv, int length);
gboolean sim_spdi_lookup(struct sim_spdi *spdi,
const char *mcc, const char *mnc);
diff --git a/src/stkutil.c b/src/stkutil.c
index 9f3bc0b..9fa7705 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -413,7 +413,7 @@ static gboolean parse_dataobj_file_list(struct
comprehension_tlv_iter *iter,
GSList **fl = user;
const unsigned char *data;
unsigned int len;
- unsigned int i;
+ unsigned int i = 1;
unsigned int start = 1;
struct stk_file *sf;
this change makes no sense to me. Please only initialize variables when
really needed. I really want the compiler to warn us when we use
variables unexpectedly.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [patch 5/6] IP support for PPP
Hi Kristen,
+/** IPCP support /
+enum {
+ /* supported codes */
+ IPCP_SUPPORTED_CODES= (1 CONFIGURE_REQUEST) |
+ (1 CONFIGURE_ACK) |
+ (1 CONFIGURE_NAK) |
+ (1 CONFIGURE_REJECT) |
+ (1 TERMINATE_REQUEST) |
+ (1 TERMINATE_ACK) |
+ (1 CODE_REJECT),
+
+ IPCP_PROTO = 0x8021,
+
+ /* option types */
+ IP_ADDRESSES= 1,
+ IP_COMPRESSION_PROTO= 2,
+ IP_ADDRESS = 3,
+ PRIMARY_DNS_SERVER = 129,
+ SECONDARY_DNS_SERVER= 131,
+};
I don't think enum is the right way for this. I can see it for the
option types, but for IPCP_PROTO and IPCP_SUPPORTED_CODES. I would say
just using simple defines is way better.
Feel free to convince me other way or show me what I have missed here.
It's mostly just a habit, but in general there are advantages to
using enum vs. a define. You are assured that the scope is kept local,
even if you are uncreative with your name choices, whereas with a macro
it is not. Also it's sometimes easier to debug with an enum vs. a macro.
I can certainly change it if you really want me too.
I have nothing against enums. I actually like them a lot if used
correctly, because gcc and gdb does make your life easier.
The thing that I dislike is clashing namespaces and scopes in just one
enum. In the example of both you mix two or more things that should at
least in different enums.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/4] Handle the conversion failure when parsing item
Hi Denis, + if (utf8 == NULL) + return FALSE; + + item-text = utf8; Why bother with utf8 variable? Just do if (item-text == NULL) return FALSE; I actually find this acceptable because 'item' is a return structure, so we should avoid modifying it in case of an error. in general it makes no difference since the assigned pointer will be the same as the previous one. However this is a minor nitpick and nothing major. So I am fine either way. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 6/6] Allow gsmdial to use gatchat ppp support
Hi Kristen,
static GAtChat *control;
static GAtChat *modem;
static GMainLoop *event_loop;
+static GAtPPP *ppp;
I hate myself for this nitpick, but please group GAtPPP *ppp with the
others before the mainloop variable.
+static void print_ip_address(guint32 ip_addr)
+{
+ struct in_addr addr;
+ addr.s_addr = ip_addr;
+ g_print(%s\n, inet_ntoa(addr));
+}
Please use print_ip_address(const char *label, guint32 ip_addr) as
mentioned in the previous review.
+ /* open ppp */
+ ppp = g_at_ppp_new(channel);
+ if (!ppp) {
+ g_print(Unable to obtain PPP object\n);
+ return;
+ }
I would call it Unable to create PPP object, because you are creating.
My understanding is the obtaining means that it would be there in the
first place. It is a nitpick.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [patch 3/6] LCP support
Hi Kristen,
Makefile.am |6 -
gatchat/gatppp.c | 53 +
gatchat/gatppp_internal.h |8 +
gatchat/gatppplcp.c | 244
++
this should be gatchat/ppp_lcp.c
+void __ppp_set_auth(GAtPPP *ppp, guint8* auth_data)
+{
+ guint16 proto = ntohs(*(guint16 *)auth_data);
This are again one of these constructs that will break on non-x86
hardware.
I think you need to create ppp_get_unaligned and ppp_put_unaligned. If
all of them are be16 anyway, you could do ppp_get_unaligned_be16 etc.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [patch 5/6] IP support for PPP
Hi Kristen,
+static guint32 bytes_to_32(guint8 *bytes)
+{
+ union addr {
+ guint8 bytes[4];
+ guint32 word;
+ } a;
+
+ memcpy(a.bytes, bytes, 4);
+ return(ntohl(a.word));
+}
This works, but is pretty ugly.
Doesn't GLib has functions to ensure retrieve unaligned data? BlueZ has
the GCC magic that is required to do this right.
I have looked everywhere for something nice from glib, but I'm not
seeing it. As far as I can tell, most people just do the bit shifting
manually -- but that's what I had originally and you didn't like it.
So I'm only seeing 2 options here, this way or the original way. If
you know of the glib function to use, please let me know what it is.
I don't see how bit shifting is gonna help you against the unaligned
access of your buffer.
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [patch 1/6] Add PPP protocol support with HDLC framing
Hi Kristen, This patch implements the basic PPP protocol. LCP, NCP etc. are handled in a different patch. --- Makefile.am |4 gatchat/gatppp.c | 133 gatchat/gatppp.h | 59 +++ gatchat/ppp.c| 455 +++ gatchat/ppp.h| 130 +++ 5 files changed, 780 insertions(+), 1 deletion(-) please keep the patches whitespace clean. Otherwise you will make it really hard and complicated for everybody if we have to do whitespace cleanup in between patches. Applying: Add PPP protocol support with HDLC framing /data/devel/ofono/.git/rebase-apply/patch:572: space before tab in indent. if (status != G_IO_STATUS_NORMAL status != G_IO_STATUS_AGAIN) /data/devel/ofono/.git/rebase-apply/patch:573: space before tab in indent. return FALSE; fatal: 2 lines add whitespace errors. Using an editor that visualizes whitespaces and tabs might help here. And empty lines at the end of a file. Please remove them and if your editor is too stupid, then please pick a different editor. Applying: Add PPP protocol support with HDLC framing /data/devel/ofono/.git/rebase-apply/patch:166: new blank line at EOF. + /data/devel/ofono/.git/rebase-apply/patch:230: new blank line at EOF. + fatal: 2 lines add whitespace errors. Before you submit any patches, you might wanna quickly check with git am that they still apply. My .gitconfig uses a strict apply policy: [diff] color = auto check = true [apply] whitespace = error [format-patch] check = true Having diff and format-patch color your whitespace damages is always a good idea. Then you see what is wrong. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 6/6] Allow gsmdial to use gatchat ppp support
Hi Kristen, Add option to use PPP to gsmdial. --- gatchat/gsmdial.c | 90 -- 1 file changed, 87 insertions(+), 3 deletions(-) Index: ofono/gatchat/gsmdial.c === --- ofono.orig/gatchat/gsmdial.c 2010-03-19 12:28:11.725601078 -0700 +++ ofono/gatchat/gsmdial.c 2010-03-19 12:43:54.427612839 -0700 @@ -29,10 +29,14 @@ #include errno.h #include unistd.h #include sys/signalfd.h +#include arpa/inet.h +#include netinet/in.h +#include net/if.h patches have to be always submitted against the latest tree. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/6] PPP Patches v3
Hi Kristen, Changes since v2 - corrected whitespace. all the patches have been applied. So form now on please send smaller patches as soon as you have them ready and tested. If you need comments or have questions, you can use the mailing list as well. some additional comments from my side. If you have variables that are only valid inside the scope of a statement, then I prefer if you only declare them there. This makes the code a bit simpler to read. Check the patch I just pushed to give you an example. Also please find access to a 32-bit system if you are running 64-bit or vice versa. Our build system is strict and will turn turn all casting mistakes in an error. Please be present on #ofono IRC as much as possible so people can report such things. Remember that your code is now part of the tree and it can break the build. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 6/6] Add PPP option to gsmdial
Hi Gustavo,
+static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ GIOChannel *channel;
+
+ if (!ok) {
+ g_print(Unable to define context\n);
+ exit(1);
I guess we should not call exit here. Just return.
it is a test program. So it doesn't really matter. We will never install
this program. It is just for the developers ;)
Regards
Marcel
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/6] PPP Patches v3
Hi Kristen, And there was another problem with one variable being guint16, but the is_option casts it back to guint8. We can't really have that. Once you start casting on that scale the compiler will not warn you about type mismatches or if the value of argument is too big for its type. Can you please let me know which git commit this fix you made was? I want to review it because all of the option types should only be a byte anyway, so I am trying to figure out if there was a mistake somewhere where we were using is_option to examine a 16 bit value. I searched through the git log and can't figure out where this change was. I can see where you changed the option type we are comparing to a regular guint to avoid compiler problems, but not the other issue you mentioned. I had a second look at these. It is the is_proto_handler actually and not the is_option. However that thing applies here. A helper function to find that handle would be better then manually coding g_list_find_custom in the functions. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/6] PPP Patches v3
Hi Kristen, And there was another problem with one variable being guint16, but the is_option casts it back to guint8. We can't really have that. Once you start casting on that scale the compiler will not warn you about type mismatches or if the value of argument is too big for its type. Can you please let me know which git commit this fix you made was? I want to review it because all of the option types should only be a byte anyway, so I am trying to figure out if there was a mistake somewhere where we were using is_option to examine a 16 bit value. I searched through the git log and can't figure out where this change was. I can see where you changed the option type we are comparing to a regular guint to avoid compiler problems, but not the other issue you mentioned. I had a second look at these. It is the is_proto_handler actually and not the is_option. However that thing applies here. A helper function to find that handle would be better then manually coding g_list_find_custom in the functions. so is_proto_handler was casting a 16 bit value to an 8 bit value? Or are you just complaining about having g_list_find_custom in the routines. It would really be easier on me to understand what you are trying to tell me if you you posted your patches to the list next time. I expect you to ensure that your patches compile on 32-bit systems. And also that they work on 32-bit and 64-bit systems. In addition I expect that little endian and big endian works smoothly. In general I don't wanna have casting in the source since it is a potential trap for errors. Without casting the compiler will warn us if we make mistakes, without we run into troubles. Especially with the different pointer sizes it is important to not cast. So in this specific cases, I think the usage of g_list_find_custom is not a good idea. Manually walking this in a helper function and ensuring that the variable and parameters types are enforced. In short summary: 1) No casting if not really really needed. Think twice before adding a casting to the source code. 2) Think about your optimization and understand what you are trying to optimize. Simple code flow is preferred. Only in hot path we should attempt to optimize manually and then it needs to be documented in the source code itself. Otherwise lets GCC do it for us. Walking the GLib list manually is sometimes a lot simpler to read than trying to use foreach type functions. 3) No variable initialization on declaration. There are a few exceptions when it makes sense. However the general rule is not to do it. We want the compiler to warn us about variables that we might use uninitialized. In most cases this indicates a mistake in the code flow and that the code is too complex. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/3] fix memory leaks in PPP
Hi Kristen, Fix a few memory leaks in PPP Kristen Carlson Accardi (3): fix memory leaks in option handling fix memory leaks after ppp_transmit fix memory leak in ppp_auth gatchat/ppp_auth.c |1 + gatchat/ppp_cp.c | 51 +-- 2 files changed, 42 insertions(+), 10 deletions(-) all three patches have been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Issue with PPP auth and empty username/password
Hi Kristen, I started tested the PPP code and run into a simple crash when not providing username and password. This should be possible. Program terminated with signal 11, Segmentation fault. #0 0x003457e7f2f2 in __strlen_sse2 () from /lib64/libc.so.6 (gdb) bt #0 0x003457e7f2f2 in __strlen_sse2 () from /lib64/libc.so.6 #1 0x0040ca9b in chap_process_challenge (priv=0x15a6bf0, new_packet=0x15a5164 \001\001) at gatchat/ppp_auth.c:86 #2 chap_process_packet (priv=0x15a6bf0, new_packet=0x15a5164 \001\001) at gatchat/ppp_auth.c:143 #3 0x0040a66f in ppp_recv (channel=value optimized out, cond=value optimized out, data=0x15a6d50) at gatchat/ppp.c:205 #4 ppp_feed (channel=value optimized out, cond=value optimized out, data=0x15a6d50) at gatchat/ppp.c:286 #5 ppp_cb (channel=value optimized out, cond=value optimized out, data=0x15a6d50) at gatchat/ppp.c:340 #6 0x003598a3923e in g_main_context_dispatch () from /lib64/libglib-2.0.so.0 #7 0x003598a3cc28 in ?? () from /lib64/libglib-2.0.so.0 #8 0x003598a3d075 in g_main_loop_run () from /lib64/libglib-2.0.so.0 #9 0x00403918 in main (argc=1, argv=0x7fff7b97eb28) at gatchat/gsmdial.c:624 Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Quick report from PPP testing
Hi Kristen, so I just downloaded the Linux kernel via the PPP connection with my Rogers' Novatel UMTS stick. : \r\n+CGREG: 2,1,FE38,EBA6\r\n\r\nOK\r\n Registered to GPRS network, roaming=False : ATD*99***1#\r : \r\nCONNECT HSDPA 7.2\r\n oops -- found acked option 2 we didn't request Unknown ipcp option type 130 oops, option wasn't acceptable Unknown ipcp option type 132 oops, option wasn't acceptable Unknown ipcp option type 130 oops, option wasn't acceptable Unknown ipcp option type 132 oops, option wasn't acceptable Unknown ipcp option type 130 oops, option wasn't acceptable Unknown ipcp option type 132 oops, option wasn't acceptable IP Address: 172.28.89.39 Primary DNS Server: 64.71.255.198 Secondary DNS Server: 64.71.255.253 The IPCP negotiation looks a bit funky. Can we get some more debug code in there. I like to know which side tried this requests. Also speeding this up would be helpful. Denis, I had to add custom terminators for this card since it reports the network speed after CONNECT string. Don't we wanna just accept every CONNECT * strings here? Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/4] add PPP_DEAD and PPP_TERMINATE support
Hi Kristen, These patches complete implementation of the PPP_DEAD and PPP_TERMINATE phases, as well as allow this to be tested via gsmdial. Kristen Carlson Accardi (4): remove unneeded debug statement add tracing for PPP terminate path separate memory cleanup from PPP shutdown gsmdial: shutdown ppp link if we have one. gatchat/gatppp.c | 25 +++-- gatchat/gsmdial.c |4 +++- gatchat/ppp.c | 27 +++ gatchat/ppp_cp.c | 10 ++ 4 files changed, 43 insertions(+), 23 deletions(-) all four patches have been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] ppp: change debug output to include control protocol prefix
Hi Kristen, gatchat/ppp_cp.c | 12 ++-- gatchat/ppp_cp.h | 10 +- gatchat/ppp_lcp.c | 12 ++-- gatchat/ppp_net.c | 12 ++-- 4 files changed, 35 insertions(+), 11 deletions(-) patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Issue with PPP auth and empty username/password
Hi Kristen, I started tested the PPP code and run into a simple crash when not providing username and password. This should be possible. Program terminated with signal 11, Segmentation fault. #0 0x003457e7f2f2 in __strlen_sse2 () from /lib64/libc.so.6 (gdb) bt #0 0x003457e7f2f2 in __strlen_sse2 () from /lib64/libc.so.6 #1 0x0040ca9b in chap_process_challenge (priv=0x15a6bf0, new_packet=0x15a5164 \001\001) at gatchat/ppp_auth.c:86 #2 chap_process_packet (priv=0x15a6bf0, new_packet=0x15a5164 \001\001) at gatchat/ppp_auth.c:143 #3 0x0040a66f in ppp_recv (channel=value optimized out, cond=value optimized out, data=0x15a6d50) at gatchat/ppp.c:205 #4 ppp_feed (channel=value optimized out, cond=value optimized out, data=0x15a6d50) at gatchat/ppp.c:286 #5 ppp_cb (channel=value optimized out, cond=value optimized out, data=0x15a6d50) at gatchat/ppp.c:340 #6 0x003598a3923e in g_main_context_dispatch () from /lib64/libglib-2.0.so.0 #7 0x003598a3cc28 in ?? () from /lib64/libglib-2.0.so.0 #8 0x003598a3d075 in g_main_loop_run () from /lib64/libglib-2.0.so.0 #9 0x00403918 in main (argc=1, argv=0x7fff7b97eb28) at gatchat/gsmdial.c:624 your patches haven't fixed this yet. Core was generated by `./gatchat/gsmdial -n /dev/ttyUSB0 -c 1 -a internet.com -l -P'. Program terminated with signal 11, Segmentation fault. #0 __strlen_sse2 () at ../sysdeps/x86_64/strlen.S:31 31 pcmpeqb (%rdi), %xmm2 (gdb) bt #0 __strlen_sse2 () at ../sysdeps/x86_64/strlen.S:31 #1 0x0040d13b in chap_process_challenge (priv=0x211aba0, new_packet=0x2119b7c \001\001) at gatchat/ppp_auth.c:86 #2 chap_process_packet (priv=0x211aba0, new_packet=0x2119b7c \001\001) at gatchat/ppp_auth.c:143 #3 0x0040a94e in ppp_recv (channel=value optimized out, cond=value optimized out, data=0x211bab0) at gatchat/ppp.c:210 #4 ppp_feed (channel=value optimized out, cond=value optimized out, data=0x211bab0) at gatchat/ppp.c:295 #5 ppp_cb (channel=value optimized out, cond=value optimized out, data=0x211bab0) at gatchat/ppp.c:347 #6 0x003598a3923e in g_main_dispatch (context=0x2118160) at gmain.c:1960 #7 IA__g_main_context_dispatch (context=0x2118160) at gmain.c:2513 #8 0x003598a3cc28 in g_main_context_iterate (context=0x2118160, block=value optimized out, dispatch=value optimized out, self=value optimized out) at gmain.c:2591 #9 0x003598a3d075 in IA__g_main_loop_run (loop=0x21183b0) at gmain.c:2799 #10 0x00403a25 in main (argc=1, argv=0x7fffa4947bf8) at gatchat/gsmdial.c:679 I notice that you finally applied my patch to allow the empty secrets. Are you still having a problem after applying that patch? actually it never made it to me. I was missing patch 1/2 from your one submission. That is why I asked you to re-send it. In the end I started going through the code and fixed it by myself. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 0/2] ppp authentication bug fixes (resend)
Hi Kristen, Here are a couple bug fixes for the PPP authentication. First patch fixes the seg fault when the secret is empty, as reported by marcel. Second, if we do fail to authenticate, we need to change ppp phases to terminate the link. Kristen Carlson Accardi (2): ppp: allow empty secret for chap challenge ppp: send PPP_FAIL when authentication fails gatchat/ppp_auth.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) this is funny. Even now patch 1/2 is missing on my side. However it seems that Denis can see it, so he took care of it. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Access to SIM card when Modem is not Powered
Hi Pekka, I've been porting the N900 modem control code to oFono. The semantics of Powered is fine with respect of the atoms, in other words, if the modem crashes and boots itself, all the atoms are flushed nicely. When powering up, the Powered can be set to true when the modem is really up and running. Do you have an overview of the different modes and transitions that the N900 modem control is using today? Not really. What do you want to know? There are some design documents describing GPIO line usage, something about SSI used for phonet messages and how modem bootloader interacts with it, and documents about the MTC design and different MTC states. this really sounds like you guys should implement RFKILL support for the Phonet subsystem. Solving this in userspace is wrong since the GPIO lines are deeply attached to specific hardware design. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: Access to SIM card when Modem is not Powered
Hi Pekka, That is also a problem. The other problem is that the party controlling the modem power state is supposed to keep GPIO lines in known position for a while after the modem has indicated it has been powered down. In an N900 running maemo, a daemon called sscd does that. sscd exits only after modem has been safely powered down during reboot and shutdown. If ofonod does the controlling, it should hang around after power off for a while, too. So I'm still having trouble understanding the issue. When oFono calls disable, the driver is expected to take all necessary steps to disable the modem. If that means waiting N seconds after the command has been sent, so be it. During shutdown of the daemon, oFonod waits for a grace period and waits on any devices that are being shut down. In effect it hangs around after power off. Another solution is to use sscd-like daemon also with ofono (the oFono Powered property would then just follow the power state of the modem). Automatic powerup is actually possible from the driver. See HFP driver for details. We reply with the busy error, you're correct. However, I don't really see anything better we can do here, do you have any suggestions? Keep the target state around somewhere, or call enable/disable regardless of the current state of the Powered property? Note that oFono does not record the powered preferences, ConnMan is responsible for that. Sending a disable when we are already disabled would be wrong and would break some plugins. And I'm still having trouble understanding why you want this. Please give concrete use-cases. Sure. I want Powered-1 that controls the atoms. Atoms should be loaded when modem is in responsive state and removed when, e.g., modem reboots. This we can do now, iow, if you connect a Nokia phone via USB, oFono can follow its state via the MTC indications it sends on top of the phonet link running over USB. I want Powered-2 that controls the modem power. When ofonod starts in N900, it should power up the internal modem. When ofonod terminates itself, it should shut down modem nicely before calling exit(). Now, enable/disable/ofono_modem_set_powered() controls both aspects; I want to separate them. It is also possible to implement Powered-2 in the probe/remove methods; however, they are quite time-consuming operations and best done from the mainloop. I am with Denis here. I am missing the point in what you are trying to achieve. The complexity you propose should not be exposed to the applications at all. This can be all handled internally. Or I am missing something essential, but right now, I don't see it. It seems to me that Marcel thinks Powered should control the RF state, too. So, a separate property for enabling he RF would be nice, too. That is what I call RFKILL and we have a proper subsystem for that. And it is different from your Power-1 and Power-2 thing? Sorry, but you really lost me now. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
PPP LCP configuration issue
Hi Kristen, so I added some debug code to actually print the options during the configuration stage. And something seems to be wrong here. lcp: pppcp_open_event: current state 0:INITIAL lcp: pppcp_up_event: current state 1:STARTING lcp: pppcp_initialize_restart_count: current state 1:STARTING lcp: pppcp_send_configure_request: current state 1:STARTING lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 lcp: pppcp_process_configure_request: current state 6:REQSENT lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 lcp: option 3 len 5 (Authentication-Protocol) c2 23 05 lcp: option 5 len 6 (Magic-Number) 01 ff ef e0 lcp: option 7 len 2 (Protocol-Field-Compression) lcp: option 8 len 2 (Address-and-Control-Field-Compression) lcp: pppcp_rcr_plus_event: current state 6:REQSENT lcp: pppcp_send_configure_ack: current state 6:REQSENT lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 lcp: option 3 len 5 (Authentication-Protocol) c2 23 05 lcp: option 5 len 6 (Magic-Number) 01 ff ef e0 lcp: option 7 len 2 (Protocol-Field-Compression) lcp: option 8 len 2 (Address-and-Control-Field-Compression) lcp: pppcp_process_configure_ack: current state 8:ACKSENT lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 oops -- found acked option 2 we didn't request lcp: pppcp_rca_event: current state 8:ACKSENT lcp: pppcp_initialize_restart_count: current state 8:ACKSENT I see the oops comment here, but we clearly requested the async control character map. So why does it get recognized as not requested? This is clearly a bug and needs to be fixed. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: PPP LCP configuration issue
Hi Kristen,
so I added some debug code to actually print the options during the
configuration stage. And something seems to be wrong here.
lcp: pppcp_open_event: current state 0:INITIAL
lcp: pppcp_up_event: current state 1:STARTING
lcp: pppcp_initialize_restart_count: current state 1:STARTING
lcp: pppcp_send_configure_request: current state 1:STARTING
lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00
lcp: pppcp_process_configure_request: current state 6:REQSENT
lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00
lcp: option 3 len 5 (Authentication-Protocol) c2 23 05
lcp: option 5 len 6 (Magic-Number) 01 ff ef e0
lcp: option 7 len 2 (Protocol-Field-Compression)
lcp: option 8 len 2 (Address-and-Control-Field-Compression)
lcp: pppcp_rcr_plus_event: current state 6:REQSENT
lcp: pppcp_send_configure_ack: current state 6:REQSENT
lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00
lcp: option 3 len 5 (Authentication-Protocol) c2 23 05
lcp: option 5 len 6 (Magic-Number) 01 ff ef e0
lcp: option 7 len 2 (Protocol-Field-Compression)
lcp: option 8 len 2 (Address-and-Control-Field-Compression)
lcp: pppcp_process_configure_ack: current state 8:ACKSENT
lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00
oops -- found acked option 2 we didn't request
lcp: pppcp_rca_event: current state 8:ACKSENT
lcp: pppcp_initialize_restart_count: current state 8:ACKSENT
I see the oops comment here, but we clearly requested the async control
character map. So why does it get recognized as not requested? This is
clearly a bug and needs to be fixed.
so I added pppdump support to GAtPPP and gsmdial. This allows us to
create log files that can be read by pppdump and Wireshark. The method
of debugging with strace is just not feasible. The attached log file is
one of these. I see a couple of confirmation requests that were send
twice in a row. That seems to be wrong. Can you explain what is going
on.
Regards
Marcel
Kµû ~ÿ}#À!}!}!} }*}}} } } } X{~Kµû U~ÿ}#À!}!} } }9}}} } } } }#}%Â#}%}%}}!}]+}'}}(}Ú~~ÿ}#À!}}!} }*}}} } } } 1}/~Kµû 8~ÿ}#À!}} } }9}}} } } } }#}%Â#}%}%}}!}]+}'}}(}}5Ö~Kµû ~ÿ!
(~Kµû D~ÿ}#À!}+}!} }(}!}]+}-}5~~ÿÂ# # Ú! Tdv®F µÞäEZEUMTS_CHAP_SRVR[¦~Kµû ~ÿÂ# 5Ü=ïW
×GÉ/ê½®äò~Kµû ~ÿÂ# y~Kµü $~ÿ!
åº~Kµü *~ÿ!
üB~Kµý $~ÿ!
=L~Kµý *~ÿ!
~Kµþ $~ÿ!
u ~Kµþ *~ÿ!
§Ë~Kµþ *~ÿ!
§Ë~Kµÿ $~ÿ!
©~Kµÿ *~ÿ!
Ä~Kµ $~ÿ!
Ôû~Kµ *~ÿ!
aK~Kµ *~ÿ!
aK~Kµ $~ÿ!
~Kµ *~ÿ!
~Kµ $~ÿ!
D_~Kµ *~ÿ!
Ñ~Kµ *~ÿ!
Ñ~Kµ $~ÿ!
Ïj~Kµ *~ÿ!
c~Kµ $~ÿ!
8~Kµ *~ÿ!
ÆQ~Kµ *~ÿ!
ÆQ~Kµ $~ÿ!
_Î~Kµ *~ÿ!
¥~Kµ $~ÿ!
~Kµ *~ÿ!
Ø~Kµ
*~ÿ!
Ø~Kµ $~ÿ!
þ+~Kµ *~ÿ!
þ~Kµ $~ÿ!
¶y~Kµ *~ÿ!
[X~Kµ
*~ÿ!
[X~Kµ $~ÿ!
n~Kµ *~ÿ!
8~Kµ $~ÿ!
Ý~Kµ *~ÿ!
Nä~Kµ *~ÿ!
Nä~Kµ $~ÿ!
xä~Kµ *~ÿ!
-¤~Kµ $~ÿ!
0¶~Kµ *~ÿ!
d~Kµ *~ÿ!
d~Kµ $~ÿ!
...@~kµ *~ÿ!
ë$~Kµ $~ÿ!
~Kµ *~ÿ!
Óí~Kµ *~ÿ!
Óí~Kµ $~ÿ!
I¥~Kµ *~ÿ!
°~Kµ $~ÿ!
÷~Kµ *~ÿ!
m~Kµ *~ÿ!
m~Kµ $~ÿ!
Ù~Kµ *~ÿ!
v-~Kµ $~ÿ!
S~Kµ *~ÿ!
t÷~Kµ *~ÿ!
Re: [PATCH] ppp: fix missing breaks in switchs
Hi Kristen, --- gatchat/ppp_cp.c |9 - 1 files changed, 4 insertions(+), 5 deletions(-) patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: PPP LCP configuration issue
Hi Kristen, so I added some debug code to actually print the options during the configuration stage. And something seems to be wrong here. lcp: pppcp_open_event: current state 0:INITIAL lcp: pppcp_up_event: current state 1:STARTING lcp: pppcp_initialize_restart_count: current state 1:STARTING lcp: pppcp_send_configure_request: current state 1:STARTING lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 lcp: pppcp_process_configure_request: current state 6:REQSENT lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 lcp: option 3 len 5 (Authentication-Protocol) c2 23 05 lcp: option 5 len 6 (Magic-Number) 01 ff ef e0 lcp: option 7 len 2 (Protocol-Field-Compression) lcp: option 8 len 2 (Address-and-Control-Field-Compression) lcp: pppcp_rcr_plus_event: current state 6:REQSENT lcp: pppcp_send_configure_ack: current state 6:REQSENT lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 lcp: option 3 len 5 (Authentication-Protocol) c2 23 05 lcp: option 5 len 6 (Magic-Number) 01 ff ef e0 lcp: option 7 len 2 (Protocol-Field-Compression) lcp: option 8 len 2 (Address-and-Control-Field-Compression) lcp: pppcp_process_configure_ack: current state 8:ACKSENT lcp: option 2 len 6 (Async-Control-Character-Map) 00 00 00 00 oops -- found acked option 2 we didn't request lcp: pppcp_rca_event: current state 8:ACKSENT lcp: pppcp_initialize_restart_count: current state 8:ACKSENT I see the oops comment here, but we clearly requested the async control character map. So why does it get recognized as not requested? This is clearly a bug and needs to be fixed. what is happening is that we are first send a Config-Request with the accm option as our only option. We then receive a Config-Request from the modem, with accm along with several other options requested. We ack the modem's Config-Request and apply the options, and then delete them from the list of options that we need to have. Then the modem response to our original Config-Request with an ack to our accm option. Because we already applied this option and removed it from the list of things we care about, it shows up as an option we didn't request. I think I could just delete the error message, and silently ignore this condition. are these options really negotiated for both sides in common. Or is the negotiation process for each side individual? Personally I like to keep such error messages. We need to cope with these cases nicely. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Add support for Option iCON 451
Hi Daniel, * Daniel Wagner daniel.wag...@bmw-carit.de [2010-04-14 07:45:11 +0200]: Signed-off-by: Daniel Wagner daniel.wag...@bmw-carit.de We do not use the signed-off line in oFono. Ah, sorry about that . Didn't know that. Do you want me to resend the patch without signoff? I just fixed that up for you manually. Just keep it in mind next time. Patch has been applied. Thanks. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Fix Let data device be optional for mbm driver
Hi Zhenhua, Dell 5530 modem has no data device port. So data device should be optional in mbm drvier. can you please include the content of /proc/bus/usb/devices for this device and the sysfs descriptions for each TTY port. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/3] hdlc: allow for scanning and escaping
Hi Kristen, PPP needs to inspect the packet protocol to see if a character should be escaped. Additionally, it needs to be able to compare against recv and xmit accm. can we just set the ACCM values instead of having an extra function call. I think that calling an extra function for every single byte is a really bad idea and the compiler can not optimize it at all. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
