Hey, On Wed, Sep 19, 2018 at 6:29 PM Bob Ham <bob....@puri.sm> wrote: > > On 16/08/18 17:30, Bob Ham wrote: > > > Possibly, yes. The SimTech (== SIMCom) plugin is using the > > MMBroadbandModemQmi class which means I either need to (1) subclass > > MMBroadbandModemQmi and mix in AT commands, or (2) use the plugin's > > AT-only MBroadbandModemSimtech class and add to it. I'd rather stick > > with QMI but I don't know how well the modem will cope with it so I need > > to experiment. > > I did stick with QMI, or at least the Qmi class. FYI, here's my patches: > > https://source.puri.sm/Librem5/ModemManager/commits/rah/sim7100-audio > > Feedback is welcome. >
Some comments: * You're subclassing the QMI modem, but then implementing AT-based logic for the Voice operations in the modem. In this case, it's better to have some "shared" code between the QMI and non-QMI modem objects, for example by the means of a "MMSharedSimtech" interface (see e.g. the XMM plugin where we are doing exactly this to share logic between MBIM and non-MBIM modem objects). * The MMCallQmiSimtech object is subclassing the MMBaseCall object, better call it MMCallSimtech instead, as there is no QMI in there. * G_DECLARE_FINAL_TYPE (GLib >= 2.44).... well, it's nice to see that, but we're currently allowing builds with GLib 2.36, so that new code would fail to build. I would possibly agree on bumping the minimum required GLib version to 2.40, which is the latest update available for Ubuntu 14.04 LTS (I maintain a PPA for 14.04 with the latest MM), at least until April 2019 which is when 14.04 is no longer maintained. * From your "FIXME: How do we ensure these are true?" question regarding MM_BASE_CALL_SUPPORTS_DIALING_TO_RINGING and MM_BASE_CALL_SUPPORTS_RINGING_TO_ACTIVE, see: https://gitlab.freedesktop.org/mobile-broadband/ModemManager/commit/6d3777fd6942fd91d132f64bfc2c9d9ee87227d1. You should set those to TRUE only after you have implemented custom support in your plugin to detect the transitions "from dialing to ringing" and "from ringing to active". The generic code in MMBaseCall does NOT support those transitions, as there is no generic URC reporting those. You should check in the SimTech AT command reference which AT command they use for these 2 transitions, and provide an implementation, in the same way as done with UCALLSTAT for u-blox modems in that patch above. Cheers! -- Aleksander https://aleksander.es _______________________________________________ ModemManager-devel mailing list ModemManager-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
