Hi Denis,

On 10/09/2019 16.10, Denis Kenzior wrote:
Hi Martin,

On 9/5/19 5:33 AM, Martin Hundebøll wrote:
The quectel M95 and MC60 modems are picky about when the sim is properly
initialized, so the logic to detect this needs to be in the quectel
plugin.

After doing basic initialization, a CPIN query is issued to detect sim
state.

If the sim is unlocked and ready, a timer is created to wait for the
modem state (AT+QINISTAT) to complete. At this point ofono is notified
about the initialized sim.

If the sim is locked, a listener for "+CPIN: READY" is set up to
know when a pin is entered. Once the indication is received, the timer
is set up as if the sim was unlocked from the beginning.

If the sim is busy/resetting, the CPIN query is issued again.
---
  plugins/quectel.c | 336 ++++++++++++++++++++++++++++++++--------------
  1 file changed, 237 insertions(+), 99 deletions(-)


So what changed in this version?

Sorry, forgot this.

I added the logic to create atoms in or after quectel_post_sim() was caled.

+    if (data->sim_state == OFONO_SIM_STATE_READY) {
+        /*
+         * when initializing with a non-locked sim card, the sim atom
+         * isn't created until now to avoid accessing it before the
+         * modem is ready.
+         *
+         * call ofono_modem_set_powered() to make ofono call
+         * quectel_pre_sim() where the sim atom is created.
+         */
+        ofono_modem_set_powered(modem, true);
+    } else {
+        /*
+         * When initialized with a locked sim card, the modem is already
+         * powered up, and the inserted signal has been sent to allow
+         * the pin to be entered. So simply update the state, and notify
+         * about the finished initialization below.
+         */
+        data->sim_state = OFONO_SIM_STATE_READY;
+    }
+
+    ofono_sim_initialized_notify(data->sim);

I don't know what you're doing here?  So sim_initialized_notify is meant to be called after PIN entry by the user, e.g. via +CPIN.  Many modems need some time to actually read the SIM file system and perform other initializations prior to functioning properly.  So oFono lets the driver tell us when this is complete.  Calling this when the PIN is locked seems wrong?

In case the sim is locked, two things are done in cpin_query():

1) we can be sure that "+CPIN: READY" appears at some point
   after a pin is entered. So we register sim_watch_cb() to
   initiate the AT+QINISTAT dance after receiving
   "+CPIN: READY".

2) Also, ofono_modem_set_powered() is called to make ofono call
   quctel_pre_sim(), so that the sim atom is created, which
   allows the pin to be entered.

Once the modem reports ready in the AT+QINISTAT response, ofono_sim_initialized_notify() is called to finish the initialization.

The other case, where the sim is not locked, the AT+QINISTAT dance is started directly from cpin_query(). Once the modem is ready, ofono_modem_set_powered() is called to create the sim atom in quectel_pre_sim(), and then ofono_sim_initialized_notify() afterwards.

Also, it isn't clear whether data->sim is valid here at this point in all cases?

I know it a bit of a hack, but I've noticed the pre_sim function is called unconditionally when doing ofono_modem_set_powered(true), so I exploited that fact here.

+
+    /*
+     * If quectel_post_sim() has not yet been called, then postpone atom
+     * creation until it is called. Otherwise create the atoms now.
+     */
+    if (data->state != QUECTEL_STATE_POST_SIM) {
+        data->state = QUECTEL_STATE_READY;
+        return;
+    }
+
+    ofono_sms_create(modem, data->vendor, "atmodem", data->aux);
+    ofono_phonebook_create(modem, data->vendor, "atmodem", data->aux);
+    ofono_voicecall_create(modem, data->vendor, "atmodem", data->aux);
+    ofono_call_volume_create(modem, data->vendor, "atmodem", data->aux);
+    data->state = QUECTEL_STATE_INITIALIZED;

I'm lost here as well?  How do you even get into this path if we're in post_sim?  Is it because you called sim_initialized when you shouldn't have and trying to take care of the weirdness?

I've tried to work-around quectel_post_sim() being called after the PIN is entered, but before I call ofono_sim_initialized_notify():

../git/drivers/atmodem/sim.c:at_crsm_read_cb() crsm_read_cb: 90, 00, 1
../git/src/simfs.c:sim_fs_op_read_block_cb() bufoff: 0, dataoff: 0, tocopy: 1
Aux: > AT+CIMI\r
../git/plugins/quectel.c:init_timer_cb() 0x610000000d40
Aux: < \r\n238013814764391\r\n\r\nOK\r\n
../git/drivers/atmodem/sim.c:at_cimi_cb() cimi_cb: 238013814764391
../git/src/sim.c:call_state_watches()
../git/src/modem.c:modem_change_state() old state: 1, new state: 2
../git/plugins/quectel.c:quectel_post_sim() 0x610000000d40

So after reading the CIMI, the SIM is marked as ready, and call_state_watches() is called. (I added a DBG() there.)

Is there a way to avoid that?

For a locked SIM you should be doing something like:

ofono_set_powered(modem, TRUE);
ofono_sim_inserted(sim, TRUE);

g_at_chat_register("CPIN: Ready", sim_ready_cb);
sim_ready_cb: poll QINISTAT.  If ready, call ofono_sim_initialized

This last part may or may not need to be synced up with the sim atom driver.

Many modems actually provide a proper unsolicited notification that makes logic like this much simpler and can be handled inside the modem driver.

The M95 modem shows "Call Ready" when QINISTAT == 2, but that is not enough. The M60 actually does show "SMS Ready" at the right time. I did ask Quectel directly to update the firmware to do QINISTAT unsolicited, but they are afraid to break exisiting users :(

Move all the atom creation into post_sim.  Voicecalls probably belong in pre_sim.

I would really like to. But to do so, I need a way to avoid post_sim being called after the PIN is entered, but before QINISTAT returns 3. Otherwise, sms is broken:

Aux: > AT+CSMS?\r
Aux: < \r\n+CMS ERROR: 3\r\n
SMS not supported by this modem. If this is an error please submit patches to support this hardware
../git/src/sms.c:sms_remove() atom: 0x604000026dd0


// Martin
_______________________________________________
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Reply via email to