Re: [PATCH_v2 2/3] cdmamodem: Create network registration atom.
Hi Marcel,
<snip>
> +#include "gatchat.h"
> +#include "gatresult.h"
> +
> +#include "common.h"
I know that you are re-using the GSM network registration constants here
and thus need common.h.
Denis, is this acceptable for you?
No, not really. Since CDMA has no defined standard API these should
probably be an enum inside cdma-netreg.h
<snip>
> +static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer
user_data)
> +{
> + struct ofono_cdma_netreg *netreg = user_data;
> + gint service_state;
> + gint service_domain;
> + gint roaming_state;
> + int status;
> + GAtResultIter iter;
> +
> + if (!ok)
> + return;
> +
> + g_at_result_iter_init(&iter, result);
> +
> + if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
> + return;
> +
> + if (!g_at_result_iter_next_number(&iter, &service_state))
> + return;
> +
> + if (!g_at_result_iter_next_number(&iter, &service_domain))
> + return;
> +
> + if (!g_at_result_iter_next_number(&iter, &roaming_state))
> + return;
> +
> + switch (service_state) {
> + case 0:
> + case 4:
Please put at least comments behind every case label so that we know
what it does mean.
> + status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
> + break;
> + case 2:
> + if (roaming_state)
> + status = NETWORK_REGISTRATION_STATUS_ROAMING;
> + else
> + status = NETWORK_REGISTRATION_STATUS_REGISTERED;
> + break;
> + default:
> + status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
> + break;
> + }
> +
> + ofono_cdma_netreg_register(netreg);
> +
> + ofono_cdma_netreg_status_notify(netreg, status);
> +}
How do we get unsolicited notification when the status changes?
And why are we using a Huawei specific command inside the generic
cdmamodem driver without at least a vendor if/switch somewhere?
Regards,
-Denis