Hi Tom,
Also, it may be clearer if the introduction of raw_read and the
refactoring of this function are done as a separate commit.
Hmm, looking at the "Quectel EC21" comment again, looks like the original code
was done to address a quirk (ie, bug???) with that modem. I'll revert to reading both.
Jonas was the one who introduced that, so he would be the one to ask :)
But, per spec, the 2 message TLVs are mutually exclusive, depending on the route action
configuration. I'm referring to setting "new_list->route[0].action =" in
get_routes_cb(). So, we shouldn't need to always read both since one is guaranteed to fail.
Also, the original code, if somehow both "notify" and "message" are NULL, it'll
erroneously use a null notify. I'll address this in the revert.
On 2nd thought, my change is still ok per that "Quectel EC21" comment. I'll keep my changes and add
Yes, it looked like it would work even for the quirky ec21, but I don't
have enough context to say for sure. Nor do I have a EC21 to test
comment to explain the 2 TLV that are being queried. Also, I introduced
raw_read because I need to pull QMI_WMS_RAW_READ out of event_notify to
service both event_notify and the message list. So, I'd like to keep
this in the same commit.
Yes exactly my point. You can have the first commit simply perform the
'pull QMI_WMS_RAW_READ out of event_notify' transformation, which is
more or less self-contained. And then the second commit would use that
new utility function in your message list code.
Smaller commits are easier to review. When I mentor new contributors I
always encourage them to split up their code as much as possible. In
this particular instance I won't insist too hard since it is likely fine
either way :)
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
