[PATCH] broadband-modem: initialize 'tac' in registration_status_check_ready()

2018-08-09 Thread Ben Chan
mm-broadband-modem.c:4395:13: error: variable 'tac' is used uninitialized 
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
if (act == MM_MODEM_ACCESS_TECHNOLOGY_LTE) {
^
mm-broadband-modem.c:4407:75: note: uninitialized use occurs here
mm_iface_modem_3gpp_update_location (MM_IFACE_MODEM_3GPP (self), lac, tac, 
cid);
  ^~~
mm-broadband-modem.c:4395:9: note: remove the 'if' if its condition is always 
true
if (act == MM_MODEM_ACCESS_TECHNOLOGY_LTE) {
^~~
mm-broadband-modem.c:4380:9: error: variable 'tac' is used uninitialized 
whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (cgreg) {
^
mm-broadband-modem.c:4407:75: note: uninitialized use occurs here
mm_iface_modem_3gpp_update_location (MM_IFACE_MODEM_3GPP (self), lac, tac, 
cid);
  ^~~
mm-broadband-modem.c:4380:5: note: remove the 'if' if its condition is always 
false
if (cgreg) {
^~~~
mm-broadband-modem.c:4280:15: note: initialize the variable 'tac' to silence 
this warning
gulong tac;
  ^
   = 0
---
 src/mm-broadband-modem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index a685e407..d1afba4d 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -4344,6 +4344,7 @@ registration_status_check_ready (MMBroadbandModem *self,
 cereg = FALSE;
 state = MM_MODEM_3GPP_REGISTRATION_STATE_UNKNOWN;
 act = MM_MODEM_ACCESS_TECHNOLOGY_UNKNOWN;
+tac = 0;
 lac = 0;
 cid = 0;
 parsed = mm_3gpp_parse_creg_response (match_info,
-- 
2.18.0.597.ga71716f1ad-goog

___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: Adding AT commands to activate call audio over USB endpoint

2018-08-09 Thread Aleksander Morgado
Hey

 We're using a SIMCom SIM7100 modem which has the ability to exchange
 call audio data over a USB serial endpoint⁰.  To enable this, the modem
 needs an AT command after the call has connected, AT+CPCMREG=1, and a
 command after call has disconnected, AT+CPCMREG=0.

>>>
>>> Would these commands be needed as soon as a call is started (even if
>>> not established)? or just when established?
>>
>> After you send ATD.  The SIMCom docs say after your receive "VOICE CALL:
>> BEGIN" but it also says "If you want hear the ring back tone, you can
>> run "AT+CPCMREG=1" command after ATDXXX;" which is what we'd want.  I've
>> tested sending it immediately after ATD and that works.
>>
>>> Also, how would it work for received calls, at which point should the
>>> command be executed?
>>
>> I believe you get "VOICE CALL: BEGIN" when you answer the call so after
>> that.
>>
>> To summarise:
>>
>> --
>>   Command| When| Call type
>> --
>> AT+CPCMREG=1 | After ATD   | outgoing
>> AT+CPCMREG=1 | After VOICE CALL: BEGIN | incoming
>> AT+CPCMREG=0 | After VOICE CALL: END   |   both
>> --
>>
>
> Ok.
>
>
>>
 Not knowing the internals of ModemManager too well, I'm wondering if
 anyone could advise on an appropriate way to add these AT commands for
 our particular modem?
>>
>>> The way to do this would be to define a new async command in the Voice
>>> interface struct (mm-iface-modem-voice.h) and then in the same voice
>>> interface implementation setup the logic to call that command whenever
>>> needed, but only if it is implemented (e.g. in this case we would have
>>> NULL defaults as others modems don't need this). Once that logic is in
>>> place, we would need a new "simcom" plugin (there is none right now),
>>> which creates a new MMBroadbandModemSimcom that implements the voice
>>> interface and subclasses the specific command.
>>
>> I'm still trying to get my head around this.  There doesn't seem to be
>> anything similar in the voice interface, can you suggest another
>> interface with some similar functionality?
>>
>
> All interfaces are really organized in the same way: they provide a
> common logic with steps that maybe subclassed by plugins. Take for
> example the Voice interface, there is a method called
> enable_unsolicited_events() in the interface. This method is
> implemented by MMBroadbandModem in some specific way (e.g. running
> +CLIP=1 to show caller number on RING), and is then subclassed by the
> u-blox plugin to not only run the parent +CLIP=1 but also to also
> enable detailed call states with +UCALLSTAT=1. In your case, all the
> core logic is probably based on the MMBroadbandModem implementation
> (unless you're using QMI... are you?) so a new "simcom" plugin could
> subclass any specific step from any of the defined interfaces.
>
>> I don't understand the setup function you suggest.  What would this do?
>> If there is a SIMCom-specific subclass for the modem which adds the
>> AT+CPCMREG calls to the existing ATD calls or "VOICE CALL" handlers,
>> what would the MMIfaceModemVoice setup function do?
>>
>
> Forget about the Voice interface, this kind of logic should go in the
> Call interface instead. The "setup" command I was suggesting was
> something that would be called in the cases you listed above, e.g.
> just after ATD is executed or just after a call is accepted. This
> would fit within the logic of the Call interface, implemented by
> MMBaseCall; check the class definition in mm-base-call.h.
>
> But if you don't want to add any new subclass-able "setup" method,
> that would be ok as well. Instead you could just create a custom
> MMCallSimcom object and subclass methods of the parent MMBaseCall
> object:
>   * Subclass MMBaseCall start() so that the parent start() is first
> called, and only then you run AT+CPCMREG=1.
>   * Subclass MMBaseCall accept() so that the parent accept() is first
> called, and only then you run AT+CPCMREG=1.
>   * Subclass MMBaseCall hangup() so that you run AT+CPCMREG=0 and then
> the parent hangup() is called.
>   * You may also need to setup URC handlers for the VOICE CALL
> notifications, e.g. if VOICE CALL: END is the only way you have to get
> reported of terminated calls, so that you call AT+CPCMREG=0 also in
> that case.
>
> My "aleksander/next" branch in git has several voice related changes,
> including the MMUbloxCall subclass to support UCALLSTAT, that may help
> a bit:
> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/tree/aleksander/next
>

Dan wrote something similar for Huawei devices, which require
AT^DDSETEX to enable the audio channel, see:
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/commit/7a4ef42d98a1672d43818d42a8c62b0b544f7812
That is not yet in git master, though.

I'm rebasing Dan's dcbw/huawei-voice 

Re: [review] Use AT+CESQ for signal interface in MBIM modems if TTY available

2018-08-09 Thread Aleksander Morgado
On Thu, Aug 9, 2018 at 4:39 PM, Aleksander Morgado
 wrote:
>>> > This MR enables support for the "Signal" interface in MBIM modems
>>> > if
>>> > they expose an AT-capable TTY port and AT+CESQ is supported. If no
>>> > AT
>>> > port is available in the device, the modem will report no extended
>>> > signal capabilities, as it was doing until now for all MBIM modems.
>>> >
>>> > https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_
>>> > requests/18
>>> >
>>>
>>> I've closed this MR because I think using the MBIM AT extensions
>>> service to get the same information fits much better in the MBIM
>>> modem
>>> object:
>>> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_re
>>> quests/25
>>
>> I think that's probably fine for now, though I would bet there will be
>> modems that don't implement ATDS and do implement CESQ in the future,
>> if they don't exist already.
>>
>
> Surprisingly, all Qualcomm and Intel based modems I've tested recently
> do support ATDS for signal and location support at least. But yes, we
> can always set it up to fallback to CESQ if it isn't supported. Let me
> prepare a new patch for that, should be easy.
>

Done now, pushed a new commit to fallback to CESQ if ATDS signal unsupported:
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_requests/25

Tested it by manually hardcoding is_atds_signal_supported to FALSE always.

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [review] Use AT+CESQ for signal interface in MBIM modems if TTY available

2018-08-09 Thread Aleksander Morgado
>> > This MR enables support for the "Signal" interface in MBIM modems
>> > if
>> > they expose an AT-capable TTY port and AT+CESQ is supported. If no
>> > AT
>> > port is available in the device, the modem will report no extended
>> > signal capabilities, as it was doing until now for all MBIM modems.
>> >
>> > https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_
>> > requests/18
>> >
>>
>> I've closed this MR because I think using the MBIM AT extensions
>> service to get the same information fits much better in the MBIM
>> modem
>> object:
>> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_re
>> quests/25
>
> I think that's probably fine for now, though I would bet there will be
> modems that don't implement ATDS and do implement CESQ in the future,
> if they don't exist already.
>

Surprisingly, all Qualcomm and Intel based modems I've tested recently
do support ATDS for signal and location support at least. But yes, we
can always set it up to fallback to CESQ if it isn't supported. Let me
prepare a new patch for that, should be easy.

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [review] Use AT+CESQ for signal interface in MBIM modems if TTY available

2018-08-09 Thread Dan Williams
On Thu, 2018-08-09 at 13:47 +0200, Aleksander Morgado wrote:
> Hey,
> 
> > 
> > This MR enables support for the "Signal" interface in MBIM modems
> > if
> > they expose an AT-capable TTY port and AT+CESQ is supported. If no
> > AT
> > port is available in the device, the modem will report no extended
> > signal capabilities, as it was doing until now for all MBIM modems.
> > 
> > https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_
> > requests/18
> > 
> 
> I've closed this MR because I think using the MBIM AT extensions
> service to get the same information fits much better in the MBIM
> modem
> object:
> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_re
> quests/25

I think that's probably fine for now, though I would bet there will be
modems that don't implement ATDS and do implement CESQ in the future,
if they don't exist already.

Dan
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [review] Use AT+CESQ for signal interface in MBIM modems if TTY available

2018-08-09 Thread Aleksander Morgado
Hey,

>
> This MR enables support for the "Signal" interface in MBIM modems if
> they expose an AT-capable TTY port and AT+CESQ is supported. If no AT
> port is available in the device, the modem will report no extended
> signal capabilities, as it was doing until now for all MBIM modems.
>
> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_requests/18
>

I've closed this MR because I think using the MBIM AT extensions
service to get the same information fits much better in the MBIM modem
object:
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_requests/25

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


[review] MBIM ATDS service based 3GPP location and extended Signal interface support

2018-08-09 Thread Aleksander Morgado
Hey,

The following MR introduces support for using the MBIM ATDS service
extensions to query for 3GPP location (LAC/TAC/CID) as well as
extended signal information (RSRP, RSRQ, RSSI...).

The MR contains 3 commits but only the last 2 are relevant. The first
one is a commit cherry-picked from the aleksander/mbim-ussd support to
avoid many conflict resolutions later on.

https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_requests/25

Comments?

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


Re: [PATCH] build: require libmbim 1.17.3

2018-08-09 Thread Aleksander Morgado
On 08/08/18 22:27, Ben Chan wrote:
> libmbim 1.17.3 adds the support for MBIM_CID_PCO, which is used by
> commit d68078b2ce0f ("broadband-modem-mbim: check if modem implements
> MBIM_CID_PCO").
> ---

Oops, this should have gone with the commit that introduced the new 
requirement. Thanks for fixing it, pushed to git master now.

>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index f400eb54..7fa4f60d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -317,7 +317,7 @@ 
> dnl-
>  dnl MBIM support (enabled by default)
>  dnl
>  
> -LIBMBIM_VERSION=1.17.1
> +LIBMBIM_VERSION=1.17.3
>  
>  AC_ARG_WITH(mbim, AS_HELP_STRING([--without-mbim], [Build without MBIM 
> support]), [], [with_mbim=yes])
>  AM_CONDITIONAL(WITH_MBIM, test "x$with_mbim" = "xyes")
> 


-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel


[review] Fix MM_SIGNAL_UNKNOWN usage

2018-08-09 Thread Aleksander Morgado
Hey,

The following MR fixes how MM_SIGNAL_UNKNOWN is used to report unset values:

https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_requests/24

I thought of deprecating the symbol and adding a new one instead,
given that we're changing the actual value of the symbol, but that
would even be more confusing for clients that may be using this
symbol. Changing the value is the easiest and cleanest way to fix it.

-- 
Aleksander
https://aleksander.es
___
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel