drivers:modem: introducing config for cereg/creg for gsm #63769

mtahirbutt · 2023-10-10T15:19:39Z

As some tests with our 4G modems shown that AT+CREG? does not work as expected with the modems. Therefore, the modem does not get internet connection as the modem expects AT+CREG=0,5 for successful PPP connection but this is never accomplished as some modems do not return AT+CREG=0,5 on successful connection to an operator rather AT+CEREG? is the right AT command to use for such modems. Therefore, a new CONFIG is added to use AT+CEREG? for such modems.
The user can select AT+CREG? or AT+CEREG? according to the type the modem supports.
This PR fixes #63917

rerickson1 · 2023-10-10T16:25:25Z

Please fix code compliance related issues.

mtahirbutt · 2023-10-10T16:52:33Z

Please fix code compliance r

Please fix code compliance related issues.

thanks sir for the comment, will address tomorrow. thanks for your patience.

emil-lindq · 2023-10-10T18:45:55Z

Just a comment, since this is a this or that, maybe this Kconfig symbol should be a choice symbol? In that case please remember to name the choice so a default choice can be selected in some overriding Kconfig.

ycsin

would it be possible to use the COND_CODE1? I just thought that it might look cleaner

ycsin

In that case please remember to name the choice so a default choice can be selected in some overriding Kconfig.

+1

mtahirbutt · 2023-10-11T09:05:49Z

In that case please remember to name the choice so a default choice can be selected in some overriding Kconfig.

thanks for the remarks, actually different manufacterers have shown different results although all of them are 4G, still I can think about it and work on it if that seems more intuitive to use in modem_gsm_type choice. thanks

This contribution addresses the support for various types of modems in gsm driver. As some 4G modems have failed to return correct output from AT+CREG?, so AT+CEREG? is the right AT command in such situation. This commit provides the possibility for user to select one type of AT command. This PR fixes zephyrproject-rtos#63917 Signed-off-by: Tahir Akram <mtahirbutt@hotmail.com>

mtahirbutt · 2023-10-15T16:55:04Z

In that case please remember to name the choice so a default choice can be selected in some overriding Kconfig.

thanks for the remarks, actually different manufacterers have shown different results although all of them are 4G, still I can think about it and work on it if that seems more intuitive to use in modem_gsm_type choice. thanks

Please review the changes. I have introduced a choice now. I have also worked with main branch, also thinking of creating a new PR with main branch. As Backport error is occuring here as I already created the issue and mention "Fix #1234"
Thanks.

ycsin · 2023-10-16T01:27:45Z

I have also worked with main branch, also thinking of creating a new PR with main branch

I dont think you can push something into a released branch like this, you will have to push things into the main branch` and do a proper backport from there

ycsin

please create another PR against main

ycsin · 2023-10-16T01:30:42Z

drivers/modem/Kconfig.gsm

+	  Use particular type of AT command for registration status.
+
+config MODEM_GSM_SELECT_CREG
+	bool "CREG command"


I think this can be a string type with "CREG" as the default value

Thanks. I tried but zephyr is not allowing me to use string as a type of choice. i do not find any example either. is there any?

oops, didn't know that, sorry, how about

config MODEM_GSM_STATUS_COMMAND string "Status command" default "CREG" if MODEM_GSM_STATUS_CMD_USE_CREG default "CEREG" if MODEM_GSM_STATUS_CMD_USE_CEREG

?

I found some similar things here: zephyrproject/modules/hal/espressif/components/esptool_py/Kconfig.projbuild

In any case, you should really create another PR against main for better documentation of this development

ycsin · 2023-10-16T01:31:02Z

drivers/modem/Kconfig.gsm

+	  Use this if the modem works only with AT+CREG?.
+
+config MODEM_GSM_SELECT_CEREG
+	bool "CEREG command"


and this, with "CEREG" as default value

ycsin · 2023-10-16T01:31:15Z

drivers/modem/Kconfig.gsm

@@ -36,6 +36,24 @@ config MODEM_GSM_QUECTEL

 endchoice

+choice MODEM_GSM_STATUS_COMMAND_TYPE


Then this can just be

Suggested change

choice MODEM_GSM_STATUS_COMMAND_TYPE

choice MODEM_GSM_STATUS_COMMAND

ycsin · 2023-10-16T01:33:06Z

drivers/modem/gsm_ppp.c

 static const struct modem_cmd check_net_reg_cmd =
+#if defined(CONFIG_MODEM_GSM_SELECT_CEREG)
+	MODEM_CMD("+CEREG: ", on_cmd_net_reg_sts, 2U, ",");


then you can do this in one liner by concatenating:
"+" "CONFIG_MODEM_GSM_STATUS_COMMAND"

drivers/modem/gsm_ppp.c

mtahirbutt · 2023-10-17T12:11:35Z

I have also worked with main branch, also thinking of creating a new PR with main branch

I dont think you can push something into a released branch like this, you will have to push things into the main branch` and do a proper backport from there

Please check the new PR #64025. All requested changes addressed. I will close this PR after a while. thanks.

rerickson1 · 2023-10-17T13:01:41Z

Moved to a new PR.

zephyrbot added the area: Modem label Oct 10, 2023

zephyrbot requested a review from rerickson1 October 10, 2023 15:27

zephyrbot assigned rerickson1 Oct 10, 2023

rerickson1 requested review from ycsin and bjarki-andreasen October 10, 2023 16:24

ycsin reviewed Oct 11, 2023

View reviewed changes

ycsin requested changes Oct 11, 2023

View reviewed changes

mtahirbutt force-pushed the gsm-modem-cereg branch 6 times, most recently from cd2bebc to ee84182 Compare October 13, 2023 10:25

mtahirbutt force-pushed the gsm-modem-cereg branch from ee84182 to 2388054 Compare October 13, 2023 11:26

ycsin reviewed Oct 16, 2023

View reviewed changes

rerickson1 closed this Oct 17, 2023

ycsin mentioned this pull request Oct 17, 2023

drivers: modem: introducing config for cereg/creg for gsm #64025

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers:modem: introducing config for cereg/creg for gsm #63769

drivers:modem: introducing config for cereg/creg for gsm #63769

mtahirbutt commented Oct 10, 2023 •

edited

Loading

rerickson1 commented Oct 10, 2023

mtahirbutt commented Oct 10, 2023

emil-lindq commented Oct 10, 2023

ycsin left a comment

ycsin left a comment

mtahirbutt commented Oct 11, 2023

mtahirbutt commented Oct 15, 2023

ycsin commented Oct 16, 2023 •

edited

Loading

ycsin left a comment

ycsin Oct 16, 2023

mtahirbutt Oct 16, 2023

ycsin Oct 16, 2023 •

edited

Loading

ycsin Oct 16, 2023

ycsin Oct 16, 2023

ycsin Oct 16, 2023

mtahirbutt commented Oct 17, 2023

rerickson1 commented Oct 17, 2023

		@@ -36,6 +36,24 @@ config MODEM_GSM_QUECTEL

		endchoice

		choice MODEM_GSM_STATUS_COMMAND_TYPE

	choice MODEM_GSM_STATUS_COMMAND_TYPE
	choice MODEM_GSM_STATUS_COMMAND

drivers:modem: introducing config for cereg/creg for gsm #63769

drivers:modem: introducing config for cereg/creg for gsm #63769

Conversation

mtahirbutt commented Oct 10, 2023 • edited Loading

rerickson1 commented Oct 10, 2023

mtahirbutt commented Oct 10, 2023

emil-lindq commented Oct 10, 2023

ycsin left a comment

Choose a reason for hiding this comment

ycsin left a comment

Choose a reason for hiding this comment

mtahirbutt commented Oct 11, 2023

mtahirbutt commented Oct 15, 2023

ycsin commented Oct 16, 2023 • edited Loading

ycsin left a comment

Choose a reason for hiding this comment

ycsin Oct 16, 2023

Choose a reason for hiding this comment

mtahirbutt Oct 16, 2023

Choose a reason for hiding this comment

ycsin Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

ycsin Oct 16, 2023

Choose a reason for hiding this comment

ycsin Oct 16, 2023

Choose a reason for hiding this comment

ycsin Oct 16, 2023

Choose a reason for hiding this comment

mtahirbutt commented Oct 17, 2023

rerickson1 commented Oct 17, 2023

mtahirbutt commented Oct 10, 2023 •

edited

Loading

ycsin commented Oct 16, 2023 •

edited

Loading

ycsin Oct 16, 2023 •

edited

Loading