[review] dcbw/qcdm-cleanups

[Dan Williams]

On Sat, 2017-04-15 at 22:48 +0200, Aleksander Morgado wrote:
> Hey Dan,
> 
> On 10/04/17 17:50, Dan Williams wrote:
> > Port some stuff over to GTask, clean up some QCDM stuff, and also
> > make
> > sure QCDM code opens the port when it needs to instead of assuming
> > there's a port already open.  That's necessary for Huawei DDSETEX
> > voice
> > support.
> > 
> 
> I've reviewed these patches:
> 
> 3297fd9b broadband-modem: open QCDM port for CDMA Serving System
> checking when needed
> 5cfa8ccc broadband-modem: open QCDM port for Call Manager state
> checking
> when needed
> 2168fb1d broadband-modem: open QCDM port for HDR state checking when
> needed
> 9b71cdd7 broadband-modem: open QCDM port for access tech checking
> when
> needed
> cb578c72 broadband-modem: open QCDM port for signal checking when
> needed
> 764cb5f9 broadband-modem-novatel: $NWRSSI can report lower values
> than -115
> 7c64a894 broadband-modem-novatel: clean up detailed registration
> state
> handling
> a8b846df broadband-modem-novatel: clean up access technology
> reporting
> 
> They all look good to me, feel free to merge them.

Thanks, merged to git master.

> Some comments:
> 
>  * Why don't we setup a pair of "mm_base_modem_qcdm_command()" and
> ""mm_base_modem_qcdm_command_full()" methods? The former would
> automatically peek a QCDM port, open it, and run the QCDM command on
> it.
> The _full() version would get as input a MMPortSerialQcdm directly.
> This
> would be similar to what's done for AT commands, and could probably
> simplify the logic as there wouldn't be any need to explicitly
> open/close the port at each QCDM command implemented.

Agreed, this would be good to do.

Dan

>  * I was thinking that it could make sense to have a common
> "task_serial_port_cleanup" method instead of the three
> hdr_state_cleanup_port(), cm_state_cleanup_port() and
> cdma1x_serving_system_state_cleanup_port(), but it probably isn't
> worth
> it, unless there are many more cases where it would apply.
> 
>