<div dir="ltr"><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">Hi,</span><div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">1) I fixed those style issues in this patch. I hope that there won't be any style issues I missed.</span></div> <div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">2) Until now, I just find model number checking way to confirm the supported modes,</span></div><div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">I also think it's kind of weird. So I will try to sync with the AT command owner of MTK</span></div> <div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">with this issue.</span></div><div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">3) I am sorry that I just can't tell you directly if "D</span>-Link DWM-156 A5" can work with this plugin.</div> <div>Because we RD just have no info of what chipset owner customer use. But I have one way that may</div><div>get some info of it. You can use COM port tool to send AT command "AT+EGMR=0,0" and it will </div><div> reply with some like "+EGMR: "MT6276"" or "+EGMR: "MT6280"". This plugin will support these devices, and you can test with it. </div><div><br></div><div>Any thing I should do to improve this MTK plugin, please let me know and i will try to do with it.</div> <div><br></div><div>Best Regards!</div><div><br style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px"> <span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">> plug-in for Modem-Manager.</span><br style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px"><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">> I have tested it on the latest Linux distro like Ubuntu 13.04, Fedora</span><br style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px"> <span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">> 18 and Debian 7.0 with MediaTek MT6280 and MT6290 devices.</span><br style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px"> <span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">> The patch and debug log (use mmcli to control modem) are in the</span><br style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px"> <span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">> attachments. I hope this plugin will be accept as part of MM, and I</span><br style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px"> <span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:14px">> will update it if there is any issue or MediaTek device change.</span><br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/9/14 Dan Williams <span dir="ltr"><<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>></span><br> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div>On Mon, 2013-09-09 at 21:07 +0800, 川恕 wrote:<br> > Hi,<br> > I am working for MediaTek and finished the first version of MTK<br> > plug-in for Modem-Manager.<br> > I have tested it on the latest Linux distro like Ubuntu 13.04, Fedora<br> > 18 and Debian 7.0 with MediaTek MT6280 and MT6290 devices.<br> > The patch and debug log (use mmcli to control modem) are in the<br> > attachments. I hope this plugin will be accept as part of MM, and I<br> > will update it if there is any issue or MediaTek device change.<br> <br> </div></div>Thanks! A couple comments:<br> <br> 1) Style issues; we try to keep a uniform code style in ModemManager.<br> This block shows two issues:<br> <br> + response = mm_base_modem_at_command_finish(MM_BASE_MODEM(self), res, &error);<br> + if(!response)<br> + {<br> + mm_dbg("Couldn't query unlock retries: '%s'", error->message);<br> + goto error_ret;<br> + }<br> <br> First, all functions should have a space between the function name and<br> the "(". Second, the { for if/else/while/for statements goes on the<br> same line as the if/else/while/for. So this should be:<br> <br> + response = mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, &error);<br> + if (!response) {<br> + mm_dbg ("Couldn't query unlock retries: '%s'", error->message);<br> + goto error_ret;<br> + }<br> <br> Also, arguments to continuation lines should align with the first line,<br> and ModemManager code only uses spaces for indentation, not tabs. So<br> for example, this:<br> <br> + if (!mm_get_int_from_match_info (match_info, 1, &pin1) ||<br> + !mm_get_int_from_match_info (match_info, 2, &pin2) ||<br> + !mm_get_int_from_match_info (match_info, 3, &puk1) ||<br> + !mm_get_int_from_match_info (match_info, 4, &puk2))<br> + {<br> + g_set_error (&error,<br> + MM_CORE_ERROR,<br> + MM_CORE_ERROR_FAILED,<br> + "Failed to parse the EPINC response: '%s'",<br> + response);<br> <br> should now become:<br> <br> + if (!mm_get_int_from_match_info (match_info, 1, &pin1) ||<br> + !mm_get_int_from_match_info (match_info, 2, &pin2) ||<br> + !mm_get_int_from_match_info (match_info, 3, &puk1) ||<br> + !mm_get_int_from_match_info (match_info, 4, &puk2))<br> + {<br> + g_set_error (&error,<br> + MM_CORE_ERROR,<br> + MM_CORE_ERROR_FAILED,<br> + "Failed to parse the EPINC response: '%s'",<br> + response);<br> <br> 2) Next, is there really no command to determine what supported modes<br> (2g/3g/4g) the device has, other than checking the model number?<br> Hopefully we can ask the device what it actually supports, instead of<br> relying on a specific model number like in get_supported_modes_ready().<br> <br> 3) is there any way to get the current access technology (GSM, GPRS,<br> EDGE, UMTS, HSDPA, HSUPA, etc) that the device is using?<br> <br> Overall the patch looks pretty good, if you can clean up the style<br> issues then I think we could apply it and work out the other details<br> later. Also, what device are you using? I've got a D-Link DWM-156 A5<br> here that has a MediaTek chipset, so hopefully I can test the plugin<br> with it.<br> <br> Thanks!<br> <span><font color="#888888">Dan<br> <br> </font></span></blockquote></div><br></div></div></div>