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.

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

Jump to bottom

feat(nm): Added modem reset timer when connection is failed #5644

Merged
merged 11 commits into from
Jan 15, 2025
Merged

feat(nm): Added modem reset timer when connection is failed #5644

merged 11 commits into from
Jan 15, 2025

Conversation

pierantoniomerlino
Copy link
Contributor

This PR adds a modem reset timer when the connection fails.

Related Issue: This PR fixes/closes N/A

Description of the solution adopted: When a cellular connection fails at the first try (most likely because of an error on the sim), the modem stays in FAILED state and NM/MM refuse to start a connection. As a result, no dbus signals are reported and the usual modem reset timer is never triggered.
This PR adds a reset timer when the connection fails. If the connection is recovered before the timeout of the timer, the task is terminated. If the connection is down, instead, a modem reset is requested.

Manual Tests: To trigger this behavior, power on a device without a sim inserted. Then try to setup a cellular connection. Observe in the logs that after Modem Reset Timeout minutes the modem is reset. Insert the sim and observe that after a further modem reset, the connection can be established.

@mattdibi mattdibi mentioned this pull request Jan 13, 2025
Merged
mattdibi
mattdibi requested changes Jan 13, 2025
View reviewed changes
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated
Comment on lines 322 to 344
private class MMFailedModemResetTimerTask extends NMModemResetTimerTask {

private final String modemManagerDbusPath;

public MMFailedModemResetTimerTask(Modem modem, String modemManagerDbusPath) {
super(modem);
this.modemManagerDbusPath = modemManagerDbusPath;
}

@Override
public void run() {
try {
MMModemState modemState = getMMModemState(modemManagerDbusPath);
if (MMModemState.MM_MODEM_STATE_FAILED.equals(modemState)) {
super.run();
}
} catch (DBusException e) {
ModemManagerDbusWrapper.logger.warn("Couldn't get state of modem interface, caused by:", e);
}
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably move where the NMModemResetTimerTask is stored (signal/handlers... or should we move both somewhere else? Extending it here is quite confusing IMO)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see... you need it here because you're leveraging the getMMModemState method. Still not super sure about it 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd create a new class instead of using the NMModemResetTimerTask one, only because I need to check the state of the modem before reset. I'd not put it in the signal/handler folder since it doesn't manage any signal, as you correclty pointed out.
Finally, since we use these classes only here for a specific task (that is we'll not use them anywhere...), I prefer to make them private.

What do you think?

kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java Outdated Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/NMDbusConnector.java Outdated Show resolved Hide resolved
kura/test/org.eclipse.kura.nm.test/src/test/java/org/eclipse/kura/nm/NMDbusConnectorTest.java Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated Show resolved Hide resolved
kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated Show resolved Hide resolved
@pierantoniomerlino
Copy link
Contributor Author

@mattdibi I updated this PR, but I've not tested it yet. So, please take a look, but don't merge it.

@pierantoniomerlino
First implementation of failed modem reset
c6f92ac 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Improved failed modem reset
6a4e945 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Minimal improvemet
14a8244 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Added test for failed modem reset handler
f809663 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Moved reset handlers to ModemManagerDbusWrapper; added tests
0b1bdb0 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Fixed copyright
26dbdcc 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Several improvements on failedModemResetTimer
ec4468d 
Signed-off-by: pierantoniomerlino <[email protected]>
@pierantoniomerlino
Changed visibility of to logger protected
ef861e0 
Signed-off-by: pierantoniomerlino <[email protected]>
mattdibi
mattdibi requested changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@mattdibi mattdibi left a comment

Choose a reason for hiding this comment

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

LGTM there's only a rename that you missed

kura/org.eclipse.kura.nm/src/main/java/org/eclipse/kura/nm/ModemManagerDbusWrapper.java Outdated
@@ -49,6 +51,7 @@ public class ModemManagerDbusWrapper {
private final DBusConnection dbusConnection;

private final Map<String, NMModemResetHandler> modemHandlers = new HashMap<>();
private final Map<String, MMFailedModemResetTimer> failedModemHandlers = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not "handlers" anymore at this point

Suggested change
private final Map<String, MMFailedModemResetTimer> failedModemHandlers = new HashMap<>();
private final Map<String, MMFailedModemResetTimer> failedModemResetTimers = new HashMap<>();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argh! 😩

@pierantoniomerlino
Changed variable name again
bf5390e 
Signed-off-by: pierantoniomerlino <[email protected]>
@mattdibi
Copy link
Contributor

mattdibi commented Jan 15, 2025 •
edited
Loading

Testing

Running current PR build on a Dg1014. I configured the modem to connect without installing a SIM. In the log I can see the following:

Jan 15 08:50:47 dynagate-10-14 ESF[866]: Settings iface "3-1.3":NM_DEVICE_TYPE_MODEM
Jan 15 08:50:47 dynagate-10-14 ESF[866]: Couldn't complete activation of 3-1.3 interface, caused by:
Jan 15 08:50:47 dynagate-10-14 ESF[866]: Modem /org/freedesktop/NetworkManager/Devices/11 in failed state or unavailable. Scheduling modem reset in 5 minutes ...
Jan 15 08:50:47 dynagate-10-14 ESF[866]: Modem /org/freedesktop/ModemManager1/Modem/0 not enabled. Enabling modem...
Jan 15 08:50:47 dynagate-10-14 ESF[866]: Unable to apply configuration to the device 3-1.3

I then installed a SIM in the slot and, after 5 minutes, I can see the following:

Jan 15 08:55:45 dynagate-10-14 ESF[866]: Cannot get bearers for modem /org/freedesktop/ModemManager1/Modem/0
Jan 15 08:55:45 dynagate-10-14 ESF[866]: 3gpp properties not found.
Jan 15 08:55:45 dynagate-10-14 ESF[866]: failed to get status for interface: 3-1.3
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Cannot get bearers for modem /org/freedesktop/ModemManager1/Modem/0
Jan 15 08:55:46 dynagate-10-14 ESF[866]: 3gpp properties not found.
Jan 15 08:55:46 dynagate-10-14 ESF[866]: failed to parse last tamper event timestamp
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Alert {"message":"\"CPU Utilization\" is below thresholds","code":"cpu","category":"performance","severity":"INFO","createdOn":1736931345686,"value":5,"threshold":85}
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Storing message on topic: #account-name/#client-id/ALERT, priority: 4
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Stored message on topic: #account-name/#client-id/ALERT, priority: 4
Jan 15 08:55:46 dynagate-10-14 ESF[866]: DataPublisherService not connected
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Published alert message: org.eclipse.kura.message.KuraPayload@49b2107a
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Alert {"message":"\"Process CPU utilization for kura\" is below thresholds","code":"cpu","category":"performance","severity":"INFO","createdOn":1736931345688,"value":18,"threshold":85}
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Storing message on topic: #account-name/#client-id/ALERT, priority: 4
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Stored message on topic: #account-name/#client-id/ALERT, priority: 4
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Published alert message: org.eclipse.kura.message.KuraPayload@67261c69
Jan 15 08:55:46 dynagate-10-14 ESF[866]: DataPublisherService not connected
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Alert {"message":"\"Cellular Signal Strength for 3-1.3\" is below critical threshold","code":"cellular","category":"connectivity","severity":"CRITICAL","createdOn":1736931346216,"value":-113,"threshold":-110}
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Storing message on topic: #account-name/#client-id/ALERT, priority: 4
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Stored message on topic: #account-name/#client-id/ALERT, priority: 4
Jan 15 08:55:46 dynagate-10-14 ESF[866]: Published alert message: org.eclipse.kura.message.KuraPayload@29dd9959
Jan 15 08:55:46 dynagate-10-14 ESF[866]: DataPublisherService not connected
Jan 15 08:55:47 dynagate-10-14 ESF[866]: Modem reset timer expired. Resetting modem /org/freedesktop/ModemManager1/Modem/0 ...
Jan 15 08:55:47 dynagate-10-14 ESF[866]: Could not perform modem reset for /org/freedesktop/ModemManager1/Modem/0 because:
org.freedesktop.dbus.exceptions.DBusExecutionException: Cannot reset the modem: operation not supported
	at jdk.internal.reflect.GeneratedConstructorAccessor68.newInstance(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Unknown Source)
	at java.base/java.lang.reflect.Constructor.newInstance(Unknown Source)
	at org.freedesktop.dbus.errors.Error.getException(Error.java:145)
	at org.freedesktop.dbus.errors.Error.throwException(Error.java:175)
	at org.freedesktop.dbus.RemoteInvocationHandler.executeRemoteMethod(RemoteInvocationHandler.java:152)
	at org.freedesktop.dbus.RemoteInvocationHandler.invoke(RemoteInvocationHandler.java:216)
	at jdk.proxy8/jdk.proxy8.$Proxy71.Reset(Unknown Source)
	at org.eclipse.kura.nm.signal.handlers.NMModemResetTimerTask.run(NMModemResetTimerTask.java:43)
	at org.eclipse.kura.nm.ModemManagerDbusWrapper$MMFailedModemResetTimerTask.run(ModemManagerDbusWrapper.java:335)
	at java.base/java.util.TimerThread.mainLoop(Unknown Source)
	at java.base/java.util.TimerThread.run(Unknown Source)

The timer does work but the current installed version of MM (1.16.2) doesn't properly support the installed modem (Quectel EM05-G)

For me this constitutes as a pass. I'll try and update the Dg1014.

Update: after updating the OS and repeating the test I get the following:

Jan 15 10:59:27 dynagate-10-14 ESF[1138]: Modem reset timer expired. Resetting modem /org/freedesktop/ModemManager1/Modem/0 ...
Jan 15 11:00:02 dynagate-10-14 ESF[1138]: New network device connected at /org/freedesktop/NetworkManager/Devices/12
Jan 15 11:00:02 dynagate-10-14 ESF[1138]: Settings iface "3-1.3":NM_DEVICE_TYPE_MODEM
Jan 15 11:00:08 dynagate-10-14 ESF[1138]: Timeout elapsed. Exiting anyway

root@dynagate-10-14:/home/guest# nmcli
eth1: connected to eth1
        "eth1"
        ethernet (imx-dwmac), 00:E0:C7:0A:E5:E5, hw, mtu 1500
        ip4 default
        inet4 192.168.1.172/24
        route4 192.168.1.0/24 metric 105
        route4 default via 192.168.1.1 metric 105

eth0: connected to eth0
        "eth0"
        ethernet (fec), 00:E0:C7:0A:E5:E6, hw, mtu 1500
        inet4 172.16.0.1/24
        route4 172.16.0.0/24 metric 104

cdc-wdm0: connecting (prepare) to kura-cdc-wdm0-connection
        "cdc-wdm0"
        gsm (qmi_wwan, option), hw

@mattdibi mattdibi merged commit 5d52466 into develop Jan 15, 2025
4 checks passed
@mattdibi mattdibi deleted the enhance_modem_reset branch January 15, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattdibi mattdibi mattdibi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Kura 6.0.0
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pierantoniomerlino @mattdibi