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

Update to ofono 1.30 #25

Open
wants to merge 127 commits into
base: master
Choose a base branch
from

Conversation

piggz
Copy link
Contributor

@piggz piggz commented Feb 13, 2022

Builds on the 1.29 PR to bring the version to 1.30.

The following changes were made
Reworked:
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=88f2ae3869a3baf12ef561449289c5c16e72ace3
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=91253a1adeed452371372eec7e338ed640ada6cd
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=4b8cc2fcac2ca962f02923a6dc30cad16506f501 (ell)
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=5d3640397abe7c2e48ec09fd3061cdfd395bab4a (ell)
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=a6001c1153b8beef99a6b432dc5c55ff325ca204
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=23a09f85593d1c63f7f8271d689ee342c8aa1047 (ell)
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=d4ced627e068e73a570da5d9e8ee451595c79c23
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=5752702cbe69f346fbbc0074b06d58f5241ed7ee (ell)

Skipped (ell):
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=74dc62b1480fa7389d73f0a1dec4c75bbb512de6
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=c92d23fab9b4b7db3c07aeceaf90504f4f3d78ed
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=3e43e3342f3fada1624a28500a5337019fed7d89
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=8bde9f095af8450cf9e8af5b31f316975f08f2f6
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=765c6655f26304c45adfdb92081448d797ce3092
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=e41252d00be587c12ef026f376aa4fa68eb54ecc
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=87932a536fb0425c2c33cf62775fd9ccb1bf8393
https://git.kernel.org/pub/scm/network/ofono/ofono.git/commit/?h=1.30&id=6a967b81c77c4364d0b6d568e494f96bbd3fffbe

d99roric and others added 30 commits March 14, 2022 17:16
In one instance it was stored as boolean and another as int.
Since its always parsed as a boolean and it is a boolean,
always store it as boolean.
Some modems, eg. Quectel EC25E, return the ESN, IMEI, and MEID even
though they support only one network type in a region. Current serial
number query gives precedence to the ESN if it exists, and does not
consider the IMEI and MEID.

Add a check of the supported radio interfaces in deciding which
serial number to return. If radio interfaces are 3GPP based, then
return the IMEI, else return the ESN. If neither exist, return MEID
if available, else fail.
The message can be emitted without the fields being present. In this case ber or rssi are 0
resulting in a null pointer deref.
Handled IPv6 address after activating PDP context.
Received IPv6 address is of format addr + netmask in the same string
in the form of "a1.a2.a3.a4.a5.a6.a7.a8.a9.a10.a11.a12.a13.a14.a15.a16.
m1.m2.m3.m4.m5.m6.m7.m8.m9.m10.m11.m12.m13.m14.m15.m16"
if (ctx->apn) is always true since apn is an array variable
if (ctx->apn) always evalues to true since it is an array member
We pass in the maximum size of the buffer to the read system call.  On
the astronomically unlikely chance that we indeed read the full buffer
full of data, the subsequent assignment will overflow it.  Fix this by
passing sizeof(buf) - 1 to the read system call instead.
In case strlen(ICCID) > 20, we simply return without freeing the ICCID
value first.
if (ctx->message_proxy) always resolves to TRUE
The default context created when provisioning fails is simply a context
with an empty APN
Jonas Bonn and others added 24 commits March 14, 2022 17:16
Some uBlox modems support multiple, simultaneously active contexts.  These
contexts are either bridged to the network interface or handled
transparently by the modem acting like a router.

The problem with this approach is that ofono and ofono clients (e.g.
mmsd) expect a dedicated _local_ network interface for each context.

As such, it doesn't make sense for ofono to set up the multiple gprs
contexts.
The TOBY L4 has no IPv6 support whatsoever.
For uBlox modems, a bit of custom setup is required, but after that the
generic "atmodem" (27.007-compatible) method implementations are
sufficient.  This driver, therefore, just puts the custom probe method
into place and defers remaining functionality to the recently exported
atmodem implementations.
The Quectel M95 and MC60 modems respond to AT+CGDATA=? with a single
+CGDATA="PPP", but the callback in gprs-context expects a list of
protocols.

Avoid falling back to the old-style ATD*99 by not expecting a list of
protocols for serial quectel modems.
The gprs-context does special casing on the quectel serial modem when
probing the supported layer 2 protocols, so pass the vendor id when
setting up the atoms.
For now the interface only exposes the modem supply voltage, but is
added as a preparation for signaling power events.
The Quectel modems issues unsolicited strings in case of power related
events. The UC15 uses +QIND: for the events, while M95 and MC60 uses
descriptive strings. (UC15 also uses a string for normal power down).

Register listeners for these strings/codes. The handler emits an
appropriate dbus signal, and closes down the modem if needed.
To detect if a context gets activated before we register
for unsolicited events we need to check if any is
already activated, and flag it auto activated.
Previously the valid "unknown" netreg status was set
during startup, but its a bit problematic for gprs.
There might be cases where a LTE context is activated
before netreg is finished updating its status.
Resulting in gprs taking faulty actions.
Instead we set the status to -1 until we are updated
with a known value.
During the time the status is -1, gprs postpones actions until
the status is valid (>= 0).
@piggz
Copy link
Contributor Author

piggz commented Mar 14, 2022

Rebased on master

for UMTS.
"lte,umts" Dual mode operation with LTE
and UMTS radio access with preference
for LTE.
Copy link
Member

Choose a reason for hiding this comment

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

That's not going to work simply like that. "umts" is already interpreted by Sailfish OS as "umts,gsm", "lte" as LTE and anything below, and it's going to stay that way. We need a different syntax for "lte only" and "umts only" modes, to keep D-Bus interface backward compatible.

I've been thinking about it couple years ago: https://github.com/monich/ofono/commits/rats

👆It's not a finished work, I need to take another look at it before I can say if it's still a valid approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let me know if there is a commit (or several) from the PR that need to be dropped.

Choose a reason for hiding this comment

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

@monich Have you had time to look into the branch yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there are good enough reasons otherwise, I'm thinking it could be better if we just switched the sailfish interpretation instead of indefinitely keeping a behavior difference to upstream.

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking backward compatibility (with 3rd party software which should survive upgrades) but since no one seems to care about it anymore then I can't think of any other reason🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great to see some action on this, hopefully we can work to updating ofono soon

@pvuorela
Copy link
Contributor

Uh, at this point not sure should we update ofono with the current setup or just reboot the whole repository with everything rebased against a real upstream version (instead of a cherry-picked almost upstream version).

@piggz
Copy link
Contributor Author

piggz commented Aug 18, 2023

@pvuorela that would be cool if you did go that way ... my understanding is that there was a desire not to use the "ell" library which is used by upstream, so thats why all this cherry-picking has happened. At this point theyre kinda 2 different projects, but if you rebased on upstream that would help out eg the Pinephone, as there is a lot of work happening just recently on the upstream qmimodem parts, so ill likely have to taken them, and do yet more PRs which rebase on 2.2 :)

@FakeShell
Copy link

any updates on this whole situation? is jolla deciding to rebase entirely or merge these prs? any response is appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.