[LEDE-DEV] enhanced 3G support
Bjørn Mork
bjorn at mork.no
Fri Dec 2 01:54:22 PST 2016
"Giuseppe Lippolis" <giu.lippolis at gmail.com> writes:
> Dear Bjørn, Matti,
>
>> I don't think it is relevant for you, though. You should continue your initial
>> idea, creating a "proto" supporting the AT managed cdc_ether based
>> modem.
>
> I add a pull request here:
>
> https://github.com/lede-project/source/pull/585
>
> Please help me to fix some issue and review it.
I'm not a user of the LEDE 3G support, so I'm unable to actually test
this. But FWIW (if anything given the above), the overall change looks
mostly good to me.
The implementation still needs some obvious cleanups before being ready
to pull, though, I didn't like the way the "connect" block was moved.
Moving it is fine if required, but commenting out the old code and
duplicating it creates a lot of cruft which just makes the review
difficult. I prefer a clean move showing identical deleted and added
lines.
And you appear to have lost the first character of each line in the
process (json => son, eval => val, etc)? This doesn't make it look like
tested....
Bjørn
More information about the Lede-dev
mailing list