Skip to content

Prevent setting COPS=0 on every connection to speed up #31

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.

Sign up for GitHub

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

Open
JohanEThomsen wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Prevent setting COPS=0 on every connection to speed up #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1433cb6
Added check for current COPS mode before overwriting
JohanEThomsen Nov 8, 2023
8f853e2
Added scan_for_operators if connect fails multiple times
JohanEThomsen Nov 9, 2023
fed85a2
Removed newlines
JohanEThomsen Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions src/base/base.toit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,22 @@ abstract class CellularBase implements Cellular:
registrations.do: session.set it [2]

if not psm:
command := operator
? COPS.manual operator.op --rat=operator.rat
: COPS.automatic
send_abortable_ session command
result := send_abortable_ session COPS.read
cur_cops := result.single
cur_mode := cur_cops[0]

// If operator is defined, do manual operator selection.
// Otherwise, only set new COPS value if it's not currently
// set to automatic mode. Calling COPS=0 every time takes
// much longer.
command := null
if operator:
command = COPS.manual operator.op --rat=operator.rat
else if cur_mode != COPS.MODE_AUTOMATIC:
command = COPS.automatic

if command:
send_abortable_ session command

wait_for_urc_ --session=session:
if done.get == "+CGREG":
Expand Down Expand Up @@ -311,17 +323,23 @@ class COPS extends at.Command:
// COPS times out after 180s, but since it can be aborted, any timeout can be used.
static MAX_TIMEOUT ::= Duration --m=3
static FORMAT_NUMERIC ::= 2

static MODE_AUTOMATIC ::= 0
static MODE_MANUAL ::= 1
static MODE_DEREGISTER ::= 2
static MODE_ONLY_FORMAT ::= 3
static MODE_MANUAL_THEN_AUTO ::= 4
static MODE_EXTENDED_SEARCH ::= 5
static MODE_EXTENDED_SEARCH_NO_TAGS ::= 6
constructor.manual operator --rat=null:
args := [1, FORMAT_NUMERIC, operator]
args := [MODE_MANUAL, FORMAT_NUMERIC, operator]
if rat: args.add rat
super.set "+COPS" --parameters=args --timeout=compute_timeout

constructor.automatic:
super.set "+COPS" --parameters=[0, FORMAT_NUMERIC] --timeout=compute_timeout
super.set "+COPS" --parameters=[MODE_AUTOMATIC, FORMAT_NUMERIC] --timeout=compute_timeout

constructor.deregister:
super.set "+COPS" --parameters=[2] --timeout=compute_timeout
super.set "+COPS" --parameters=[MODE_DEREGISTER] --timeout=compute_timeout

constructor.scan:
super.test "+COPS" --timeout=compute_timeout
Expand Down
10 changes: 10 additions & 0 deletions src/base/service.toit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import .cellular
import ..api.state

CELLULAR_FAILS_BETWEEN_RESETS /int ::= 8
CELLULAR_FAILS_UNTIL_SCAN /int ::= 2

CELLULAR_RESET_NONE /int ::= 0
CELLULAR_RESET_SOFT /int ::= 1
Expand Down Expand Up @@ -158,6 +159,15 @@ abstract class CellularServiceProvider extends ProxyingNetworkServiceProvider:
logger.info "enabling radio"
driver.enable_radio
logger.info "connecting"
// After the CELLULAR_FAILS_UNTIL_SCAN threshold has passed,
// we initiate a scan every third attempt to limit collisions
// with reset attempts.
if (attempts_ > CELLULAR_FAILS_UNTIL_SCAN) and (attempts_ % 3 == 0):
logger.info "scanning for operators" --tags={"attempt": attempts_}
operators := driver.scan_for_operators
Copy link
Copy Markdown
Contributor

@kasperl kasperl Nov 10, 2023

Choose a reason for hiding this comment

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

So we ignore the returned operators, but we believe that doing the scan will somehow have a side-effect on the next automatic attempt?

Copy link
Copy Markdown
Contributor Author

@JohanEThomsen JohanEThomsen Nov 10, 2023

Choose a reason for hiding this comment

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

Yes that's the idea. It's not clearly described in the documentation, but our tests suggests that the scan updates the internal caches of modem's list of available operators, which then affects how it decides on a given operator in the subsequent automatic connect.

We have seen a device, which consistently kept re-choosing a network, where registration succeeded but the following PDP context activation failed, such that it never achieved a proper connection. After a couple of scans, it chose another operator and thus automatically achieved proper connection again.

However, it would be better to keep a flashed-back list of the used operators in the driver so we can prevent the situation completely (like the TODO comment suggests).

// TODO: Track which operators always fail and blacklist them
// or select other ones manually to prevent reselection of
// non-functional operators.
driver.connect
update-attempts_ 0 // Success. Reset the attempts.
logger.info "connected"
Expand Down