Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gen3] Allow UMNOPROF SIM_SELECT to default back to SW_DEFAULT #2211

Merged
merged 2 commits into from
Oct 9, 2020

Conversation

technobly
Copy link
Member

Problem

3rd party SIMs that are not detected with a UMNOPROF, do not default to the SW_DEFAULT profile (0), but get stuck in an init loop in Gen 3.

On Gen 2, it will default to SW_DEFAULT profile, but it will take 8 tries before it does taking up unnecessary time.

Solution

  • [gen2] A repeated UMNOPROF SIM_SELECT allows MDMParser::init to continue early [ch64292]
  • [gen3] Allow UMNOPROF SIM_SELECT to default back to SW_DEFAULT [ch64292]

Steps to Test

  • Run the example app
  • Let the device connect to the cloud with a Particle SIM
  • Triple tap the Mode button and wait for "Cold boot for a Factory Boot Experience!"
  • Power off the device
  • Switch the SIM for a 3rd party SIM
  • Power on the device and it should connect quickly using the SW_DEFAULT profile.

NOTE: You will see a log that says the following if this happens:

UMNOPROF=1 did not resolve a built-in profile, please check if UMNOPROF=100 is required!

This is up to the user to research and implement. YMMV on the exact configuration needed. Please contact your SIM provider.

Example App

#include "Particle.h"
Serial1LogHandler logHandler(115200, LOG_LEVEL_ALL);
SYSTEM_MODE(SEMI_AUTOMATIC);
bool reset_umnoprof = false;
void button_handler(system_event_t event, int clicks)
{
    if (event == button_final_click) {
        if (clicks == 3) {
            reset_umnoprof = true;
        }
    }
}
void setup() {
    System.on(button_final_click, button_handler);
    // Cellular.setActiveSim(EXTERNAL_SIM); // you might need this
    // Cellular.setActiveSim(INTERNAL_SIM);
    Particle.connect();
}

void loop() {
    if (reset_umnoprof) {
        reset_umnoprof = false;

        // System.reset();
        Cellular.command(60000, "AT+CFUN=0\r\n");
        Cellular.command(1000, "AT+UMNOPROF=0\r\n");
        Cellular.command(1000, "AT+UMNOPROF?\r\n");
        Cellular.command(1000, "AT+CFUN=15\r\n");
        Log.info("\r\n\r\nCold boot for a Factory Boot Experience!\r\n\r\n");
    }
}

References

ch64292


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • N/A Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@technobly technobly added the bug label Oct 8, 2020
@technobly technobly added this to the 2.0.0 milestone Oct 8, 2020
if (netConf_.netProv() == CellularNetworkProvider::TWILIO) { // if Twilio Super SIM
umnoprof = static_cast<int>(UbloxSaraUmnoprof::STANDARD_EUROPE);
} else {
// break out of do-while if we're trying to set SIM_SELECT a second time
if (resetCount >= 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the PR description, shouldn't this workaround only work when the external SIM is used? Or is it not that important to have a correct profile selected for officially supported providers other than Twilio?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the only other SIM we only support on R410 is Kore AT&T, which does get picked up via the SIM_SELECT as AT&T profile. SIM_SELECT is used as a way to somewhat provide 3rd party SIM support if it works... if not... then it will default now to SW_DEFAULT 0 profile.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other words, it's ok if the client is unable to set SIM_SELECT for Kore on the first try, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe make it try at least twice?

Copy link
Member

@sergeuz sergeuz Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would stick to the previous behavior as much as possible while still addressing the very specific issue the customer is having. If I'm not mistaken 2.0.0 is the first release that introduces this reset-and-retry loop when setting the operator profile, and, most importantly, it also fails the initialization if it wasn't able to set the profile after a number of attempts, which is exactly what is causing the issue.

I'm assuming that there's a good reason why we introduced this loop, so what if after making a few attempts to set the profile we simply ignore the fact that the profile wasn't set correctly and proceed with the initialization just like previous Device OS versions did?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be fine with it, since I'm almost certainly missing some context here 😅 I just wanted to make sure this is exactly the behavior we wanted

Copy link
Member Author

@technobly technobly Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to merge this as it worked mostly ok as-is, but with the other UMNOPROF=100 PR and these changes together I'm noticing some more cases that need to be addressed. So we can pick up the review over there :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I have an ask: if this is going to land in 2.0.0 as it is implemented now, let's add a comment that would state that it's an intended behavior, as that's not obvious from the code (hence why it caught my eye).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will change, but I have two PR's open for similar parts of the code which is making life difficult. Since that other one has more changes... I'll merge this one, rebase that one and make the changes. I'll try to make it very obvious what the intended behavior is with comments.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Thank you 🙏

@technobly technobly changed the title Fix/ch64292 umnoprof [gen3] Allow UMNOPROF SIM_SELECT to default back to SW_DEFAULT Oct 9, 2020
@technobly technobly merged commit fea5eb0 into develop Oct 9, 2020
@technobly technobly deleted the fix/ch64292-umnoprof branch October 9, 2020 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants