simtech/mm-broadband-modem-simtech.c: In function ‘enable_unsolicited_events_context_step’:
simtech/mm-broadband-modem-simtech.c:442:18: error: this statement may fall through [-Werror=implicit-fallthrough=]
442 | ctx->step++;
| ~~~~~~~~~^~
simtech/mm-broadband-modem-simtech.c:444:5: note: here
444 | case ENABLE_UNSOLICITED_EVENTS_STEP_LAST:
| ^~~~
Useful when clamping a unsigned integer with low threshold set to 0,
which would give us compiler warnings with -Wtype-limits when using
CLAMP(), e.g.:
via/mm-broadband-modem-via.c: In function ‘handle_evdo_quality_change’:
/usr/include/glib-2.0/glib/gmacros.h:811:63: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
811 | #define CLAMP(x, low, high) (((x) > (high)) ? (high) : (((x) < (low)) ? (low) : (x)))
| ^
via/mm-broadband-modem-via.c:284:19: note: in expansion of macro ‘CLAMP’
284 | quality = CLAMP (quality, 0, 100);
| ^~~~~
The SIM7600E ends up emitting these URCs with too many <CR>s, and the
generic +CRING handler doesn't catch them, interfering with other
actions, e.g.:
$ sudo mmcli --call 1 --accept
error: couldn't accept the call: 'GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Failed: Couldn't accept the call: Unhandled response '+CRING: VOICE
+CRING: VOICE''
Just processing and parsing them for now, so that they don't interfere
with other commands:
$ sudo mmcli --call 1 --accept
error: couldn't accept the call: 'GDBus.Error:org.freedesktop.ModemManager1.Error.Core.Failed: Couldn't accept the call: Unhandled response '
VOICE CALL: BEGIN''
We will explicitly query for +AUTOCSQ and +CNSMOD support before
trying to enable them.
If +AUTOCSQ is supported and correctly enabled, we'll request signal
quality polling to be disabled.
If +CNSMOD is supported and correctly enabled, we'll request access
technology polling to be disabled.
The +AUTOCSQ setup enables automatic signal quality reporting via +CSQ
URCs, which unfortunately have the same format as the +CSQ command
responses.
Therefore, we need to subclass load_signal_quality() explicitly in
order to ignore those cases where the response to the +CSQ command is
received empty (already processed as a URC).
The logic to setup the generic +CIND indications in the SIM7600E seems
to end up breaking connectivity when in TTY+PPP mode. If we issue the
AT+CMER=3,0,0,1 command, this will end up in the modem not replying a
correct CONNECT response to the ATD command.
Let's disable these +CIND indications completely in the simtech
plugin, as there is even no AT command reference saying those are
supported.
Fixes https://gitlab.freedesktop.org/mobile-broadband/ModemManager/issues/144
From 5s to 20s, as it seems it could take much more to complete and
get a response, as seen in the logs below.
<debug> [1568553228.546862] (ttyUSB3): --> 'AT+CNSMOD=1<CR>'
<debug> [1568553233.799470] (ttyUSB3): --> 'AT+AUTOCSQ=1,1<CR>'
<debug> [1568553238.798866] (ttyUSB3) setting up 3GPP unsolicited registration messages handlers
<debug> [1568553238.798932] (ttyUSB2) setting up 3GPP unsolicited registration messages handlers
<debug> [1568553238.798990] (ttyUSB3) device open count is 1 (close)
<warn> [1568553238.799029] (tty/ttyUSB3) at port timed out 2 consecutive times
<debug> [1568553238.799094] (ttyUSB3) device open count is 2 (open)
<debug> [1568553238.799148] (ttyUSB3): --> 'AT+CREG=2<CR>'
<warn> [1568553241.798727] (tty/ttyUSB3) at port timed out 3 consecutive times
<debug> [1568553241.798799] (ttyUSB3): --> 'AT+CREG=1<CR>'
<debug> [1568553244.795389] Enabling unsolicited registration events in primary port failed: 'AT sequence failed'
<debug> [1568553244.795462] (ttyUSB2) device open count is 2 (open)
<debug> [1568553244.795507] (ttyUSB3) device open count is 1 (close)
<warn> [1568553244.795545] (tty/ttyUSB3) at port timed out 4 consecutive times
<debug> [1568553244.795588] (ttyUSB2): --> 'AT+CREG=2<CR>'
<debug> [1568553244.800815] (ttyUSB3): <-- '<CR><LF>OK<CR><LF><CR><LF>OK<CR><LF>'
<debug> [1568553244.801624] (ttyUSB3): <-- '<CR><LF>OK<CR><LF><CR><LF>OK<CR><LF>'
<debug> [1568553244.808710] (ttyUSB2): <-- '<CR><LF>OK<CR><LF>'
We keep the pflags input in mm_base_modem_grab_port() so that plugins
can use other methods to gather port type hints (e.g. querying with AT
commands as in Huawei/Telit or looking at sysfs properties as in HSO).
For standard udev tag port type hints, it will be the base modem
looking them up.
Note that there is no longer any need to ignore non-flagged ports for
those modems that require primary/secondary flags. They will be
implicitly ignored when mm_base_modem_organize_ports() decides which
ports to use, as the flagged ones are preferred over the non-flagged
ones.
We define 3 common udev tag ids to be used by all plugins:
* ID_MM_PORT_TYPE_AT_PRIMARY: the primary modem port. It will be used
for AT control and also as PPP if there is no other port flagged
explicitly to do PPP.
* ID_MM_PORT_TYPE_AT_SECONDARY: the secondary modem port. It will be
used when/if the primary port gets connected to do PPP.
* ID_MM_PORT_TYPE_PPP: the port to be used to do PPP only. This tag
makes sense only when the primary port shouldn't be used for PPP,
i.e. when there is a port dedicated to do PPP and one port
dedicated for control.
When a new USB device is hotplugged, e.g. a USB<->RS232 converter that
exposes a single ttyUSB0, these udev events happen:
add /devices/pci0000:00/0000:00:14.0/usb2/2-1 (usb/usb-device)
add /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0 (usb/usb-interface)
add /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0/ttyUSB0 (usb-serial)
add /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0/ttyUSB0/tty/ttyUSB0 (tty)
bind /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0/ttyUSB0 (usb-serial)
bind /devices/pci0000:00/0000:00:14.0/usb2/2-1/2-1:1.0 (usb/usb-interface)
bind /devices/pci0000:00/0000:00:14.0/usb2/2-1 (usb/usb-device)
Our udev rules in MM only added tags in the 'add' events, and it looks
like the only ones 'persistent' after this sequence are those of the
last event happening on the specific path.
This meant that all TTY subsystem rules (e.g. ID_MM_CANDIDATE) would
be stored for later check (e.g. if ModemManager is started after these
rules have been applied), which was ok. "udevadm info -p ..." would
show these tags correctly always.
But this also meant that the 'bind' udev event happening for the USB
device didn't get any of our device-specific tags, and so we would be
missing them (e.g. ID_MM_DEVICE_MANUAL_SCAN_ONLY) if MM is started
after the last event has happened. "udevadm info -p ..." would
not show these tags.
Modify all our rules to also run at the 'bind' events.
See, for context:
https://github.com/systemd/systemd/issues/8221
There are 2 main types of udev properties: device-specific and
port-specific.
The port-specific properties are set independently per port (e.g. port
type hints set per interface number for a given vid:pid).
The device-specific properties apply to all ports in the device. Some
of these properties are currently expected in the physical device
(e.g. ID_MM_PLATFORM_DRIVER_PROBE) while some others are expected in
each port (e.g. the plugin udev tag filters).
This patch tries to simplify the logic and just assume that the device
specific tags may be given in either the physical device or the port
device, by providing separate APIs to retrieve port-specific or
device-specific (global) properties. If the same tag is given in both
the device and the port, the one in the device takes preference.
For the generic backend, these new APIs are really useless, as all
device-specific and port-specific properties are always stored in the
port object themselves (there is no 'tree' of devices in the generic
backend, no 'physdev' device).
For the udev backend, though, there really is a difference, as the
tags may be set in port or device.
https://bugs.freedesktop.org/show_bug.cgi?id=100156
The mm_base_modem_grab_port() now receives a MMKernelDevice directly from the
plugin, which is then stored in the MMPort corresponding to the port.
This means that we have direct access to e.g. all properties set by udev rules
everywhere, and we don't need additional GUdevClient objects (e.g. like the one
used in the Huawei plugin to detect NDISDUP support during runtime).
For virtual ports (e.g. generated during unit tests), we have a new 'generic'
kernel device object which just provides the values from the kernel device
properties given during its creation.
Instead of relying constantly on GUdevDevice objects reported by GUdev, we now
use a new generic object (MMKernelDevice) for which we provide an initial GUdev
based backend.
All ports of the same modem reported by the kernel will all be associated with
a common 'uid' (unique id), which uniquely identifies the physical device. This
logic was already in place, what we do now is avoid calling it the 'sysfs
path' of the physical device, because we may not want to use that to identify
a device.
This logic now also enables the possibility of "naming" the modems in a unique
way by setting the "ID_MM_PHYSDEV_UID" property in the "usb_device" that owns
all the ports.
E.g. a custom device has 4 modems in 4 different USB ports. The device path of
each USB device will always be the same, so the naming rules could go like this:
$ vim /usr/lib/udev/rules.d/78-mm-naming.rules
ACTION!="add|change|move", GOTO="mm_naming_rules_end"
DEVPATH=="/devices/pci0000:00/0000:00:1d.0/usb4/4-1/4-1.5/4-1.5.1", ENV{ID_MM_PHYSDEV_UID}="USB-MODEM-1"
DEVPATH=="/devices/pci0000:00/0000:00:1d.0/usb4/4-1/4-1.5/4-1.5.2", ENV{ID_MM_PHYSDEV_UID}="USB-MODEM-2"
DEVPATH=="/devices/pci0000:00/0000:00:1d.0/usb4/4-1/4-1.5/4-1.5.3", ENV{ID_MM_PHYSDEV_UID}="USB-MODEM-3"
DEVPATH=="/devices/pci0000:00/0000:00:1d.0/usb4/4-1/4-1.5/4-1.5.4", ENV{ID_MM_PHYSDEV_UID}="USB-MODEM-4"
LABEL="mm_naming_rules_end"
Each of the modems found will have a unique UID retrieved from the previous list
of rules. Then, "mmcli" has also been updated to allow using the UID instead of
the modem DBus path or index, e.g.:
$ sudo mmcli -m USB-MODEM-1
/org/freedesktop/ModemManager1/Modem/0 (device id '988d83252c0598f670c2d69d5f41e077204a92fd')
-------------------------
Hardware | manufacturer: 'ZTE CORPORATION'
| model: 'MF637'
| revision: 'BD_W7P673A3F3V1.0.0B04'
| supported: 'gsm-umts'
| current: 'gsm-umts'
| equipment id: '356516027657837'
-------------------------
System | device: 'USB-MODEM-1'
| drivers: 'option'
| plugin: 'ZTE'
| primary port: 'ttyUSB5'
| ports: 'ttyUSB5 (at)'
...
$ sudo mmcli -m USB-MODEM-1 --enable
...
Commit 7ff57f9808 introduced a change to try to
use ATTRS{bInterfaceNumber} as a common way to match by interface number, but
this logic is broken because all the rules that we use to match by interface
number (attribute in the interface device) also require matching by idVendor
and idProduct (attributes in the physdev device), and udev rules forbid matches
from more than one parent device at a time.
We could use ATTR{bInterfaceNumber} (instead of ATTRS) to tag the actual USB
interface device, but that would require a change in all the plugins to look for
the tag not in the TTY device, but in its parent.
So, recover the original behavior, where a hidden property is created containing
the first bInterfaceNumber found in the list of parent devices, and then run
the matches against idVendor and idProduct only if the hidden property is found
with the expected value.
