[OpenWrt-Devel] umbim: add registration set support
diff mbox series

Message ID 20190110145800.63138-2-feinerer@logic.at
State Accepted
Headers show
Series
  • [OpenWrt-Devel] umbim: add registration set support
Related show

Commit Message

Ingo Feinerer Jan. 10, 2019, 2:58 p.m. UTC
This implements the MBIM automatic registration mode to let the function
select the best provider network.

Signed-off-by: Ingo Feinerer <feinerer@logic.at>
---
 cli.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Bjørn Mork Feb. 14, 2019, 9:36 a.m. UTC | #1
Ingo Feinerer <feinerer@logic.at> writes:

> anyone willing to review/commit this diff?
>
> http://lists.infradead.org/pipermail/openwrt-devel/2019-January/015445.html
> http://lists.infradead.org/pipermail/openwrt-devel/2019-January/015444.html


I assume John is very busy based on currently observed activity.  So you
may have to be patient...

The patch is registered in patchwork, so it's safely queued and will not
be lost: http://patchwork.ozlabs.org/patch/1022983/

FWIW, your patch looks good to me.  It could probably be improved with
actual argument parsing, maybe allowing setting MBIM_REGISTER_ACTION_MANUAL too.
But I see no reason why that can't be added in a later followup if/when
someone actually needs it.

My test modem (EM7455) defaults to auto, so there isn't much difference
in the result with and without your patch.  But just to display the SET
vs QUERY:


bjorn@miraculix:~$ umbim -v -d /dev/cdc-wdm0 -n -t 42 registration auto
sending (64): 03 00 00 00 40 00 00 00 2a 00 00 00 01 00 00 00 00 00 00 00 a2 89 cc 33 bc bb 8b 4f b6 b0 13 3e c2 aa e6 df 09 00 00 00 01 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
  header_type: 0003
  header_length: 0040
  header_transaction: 002A
reading (124): 03 00 00 80 7c 00 00 00 2a 00 00 00 01 00 00 00 00 00 00 00 a2 89 cc 33 bc bb 8b 4f b6 b0 13 3e c2 aa e6 df 09 00 00 00 00 00 00 00 4c 00 00 00 00 00 00 00 03 00 00 00 01 00 00 00 20 00 00 00 01 00 00 00 30 00 00 00 0a 00 00 00 3c 00 00 00 0e 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 32 00 34 00 32 00 30 00 31 00 00 00 54 00 45 00 4c 00 45 00 4e 00 4f 00 52 00 00 00 
  header_type: 80000003
  header_length: 007C
  header_transaction: 002A
  command_id: 0009
  status_code: 0000
  nwerror: 0000 - unknown
  registerstate: 0003 - home
  registermode: 0001 - automatic
  availabledataclasses: 0020 - lte
  currentcellularclass: 0001 - gsm
  provider_id: 24201
  provider_name: TELENOR
  roamingtext: (null)

bjorn@miraculix:~$ umbim -v -d /dev/cdc-wdm0 -n -t 42 registration
sending (48): 03 00 00 00 30 00 00 00 2a 00 00 00 01 00 00 00 00 00 00 00 a2 89 cc 33 bc bb 8b 4f b6 b0 13 3e c2 aa e6 df 09 00 00 00 00 00 00 00 00 00 00 00 
  header_type: 0003
  header_length: 0030
  header_transaction: 002A
reading (124): 03 00 00 80 7c 00 00 00 2a 00 00 00 01 00 00 00 00 00 00 00 a2 89 cc 33 bc bb 8b 4f b6 b0 13 3e c2 aa e6 df 09 00 00 00 00 00 00 00 4c 00 00 00 00 00 00 00 03 00 00 00 01 00 00 00 20 00 00 00 01 00 00 00 30 00 00 00 0a 00 00 00 3c 00 00 00 0e 00 00 00 00 00 00 00 00 00 00 00 02 00 00 00 32 00 34 00 32 00 30 00 31 00 00 00 54 00 45 00 4c 00 45 00 4e 00 4f 00 52 00 00 00 
  header_type: 80000003
  header_length: 007C
  header_transaction: 002A
  command_id: 0009
  status_code: 0000
  nwerror: 0000 - unknown
  registerstate: 0003 - home
  registermode: 0001 - automatic
  availabledataclasses: 0020 - lte
  currentcellularclass: 0001 - gsm
  provider_id: 24201
  provider_name: TELENOR
  roamingtext: (null)


Don't know if the OpenWrt patchworks collects these, but in case it does:

Reviewed-by: Bjørn Mork <bjorn@mork.no>



Bjørn
Bjørn Mork April 9, 2019, 9:03 p.m. UTC | #2
Bjørn Mork <bjorn@mork.no> writes:
> Ingo Feinerer <feinerer@logic.at> writes:
>
>> anyone willing to review/commit this diff?
>>
>> http://lists.infradead.org/pipermail/openwrt-devel/2019-January/015445.html
>> http://lists.infradead.org/pipermail/openwrt-devel/2019-January/015444.html
>
>
> I assume John is very busy based on currently observed activity.  So you
> may have to be patient...
>
> The patch is registered in patchwork, so it's safely queued and will not
> be lost: http://patchwork.ozlabs.org/patch/1022983/

Is there anything I or anyone else can do to help here?

There are far too many patches like this one just sitting in patchwork.
Pull requests on github works really well for the repos there. Thanks a
lot to all the hard working reviewers/committers! But stuff like umbim
here, and the other repos only avilable on git.openwrt.org, seems to
depend on single maintainers. That is a problem for a volunteer
project. You can't depend on people having infinite spare time.  Very
few have...

Definitely not blaming anyone for lack of time. But is there any chance
someone else can push a few patches to umbim.git?  And maybe look
through patchwork for anything else currently falling through the
cracks?


Thanks,

Bjørn
Petr Štetiar April 10, 2019, 10:09 a.m. UTC | #3
Bjørn Mork <bjorn@mork.no> [2019-04-09 23:03:56]:


Hi Bjørn,

> Is there anything I or anyone else can do to help here?

sure, for me it's very helpful to have at least some Reviewed-by/Tested-by
tags. In case of Tested-by it's also good to know some details about the test
setup as well (device, settings etc.). This tags helps to prioritize as well.

> There are far too many patches like this one just sitting in patchwork.

It's about 108 patches, some of them already delegated, some of them RFCs,
which makes it down to roughly about 50 patches. There is about 101 PRs on
GitHub and roughly 810 open tasks/issues in Flyspray as well. So yeah, a lot
of work.

> Pull requests on github works really well for the repos there. 

Well, although I'm personally not such big fan of this platform for open
source projects (yeah, we should promote usage of other FS/OSS), it makes
handling of the huge amount of patches much more convenient, so I can live
with that.  Patchwork is nice till you can keep with the pace of new patches,
once there's some backlog it gets awkward.

> That is a problem for a volunteer project. You can't depend on people having
> infinite spare time.  Very few have...

Good tooling could help with missing human resources a lot. I wish the day had
64 hours and we could've something like kernelci.org, i.e. have more automagic
testing and so on.

> But stuff like umbim here, and the other repos only avilable on
> git.openwrt.org, seems to depend on single maintainers.

It might look like that, but from my point of view it's just because
umbim/mbim/uqmi are projects which are domain specific, so probably not so
easy candidates for review, testing and merging. BTW anybody with commit
access can push this patches.

Anyway, I've to admit, that it's really PITA to work in this fragmented
environment (Patchwork, GitHub, Flyspray) and I'm going to bring this topic on
the developer meeting in June.

> But is there any chance someone else can push a few patches to umbim.git?

Actually I've missed this patch while going through the patchwork list this
week, so thank you for the reminder.

I've merged this and the other mbim proxy patch in my staging repo[1] and
added two more on the top (adding -Wextra compiler check), which I've sent to
the list for the review just a few moments ago. 

So it would be nice if you could take a look and review them before I'm going
to bother blogic for his ACK (not strictly necessary, but nice to have) in
order to push it.

Thanks.

1. https://github.com/ynezz/umbim/tree/staging

-- ynezz
Bjørn Mork April 10, 2019, 1:56 p.m. UTC | #4
Petr Štetiar <ynezz@true.cz> writes:

>> But is there any chance someone else can push a few patches to umbim.git?
>
> Actually I've missed this patch while going through the patchwork list this
> week, so thank you for the reminder.
>
> I've merged this and the other mbim proxy patch in my staging repo[1] and
> added two more on the top (adding -Wextra compiler check), which I've sent to
> the list for the review just a few moments ago. 
>
> So it would be nice if you could take a look and review them before I'm going
> to bother blogic for his ACK (not strictly necessary, but nice to have) in
> order to push it.

Thanks!

And good luck with the tools discussion.  It's good to know that these
things are on the radar.



Bjørn

Patch
diff mbox series

diff --git a/cli.c b/cli.c
index 1dd6330..e00b6d4 100644
--- a/cli.c
+++ b/cli.c
@@ -297,7 +297,16 @@  mbim_pin_state_request(void)
 static int
 mbim_registration_request(void)
 {
-	mbim_setup_command_msg(basic_connect, MBIM_MESSAGE_COMMAND_TYPE_QUERY, MBIM_CMD_BASIC_CONNECT_REGISTER_STATE, 0);
+	if (_argc > 0) {
+		struct mbim_basic_connect_register_state_s *rs =
+			(struct mbim_basic_connect_register_state_s *) mbim_setup_command_msg(basic_connect,
+					MBIM_MESSAGE_COMMAND_TYPE_SET, MBIM_CMD_BASIC_CONNECT_REGISTER_STATE,
+					sizeof(struct mbim_basic_connect_register_state_s));
+
+		rs->registeraction = htole32(MBIM_REGISTER_ACTION_AUTOMATIC);
+	} else {
+		mbim_setup_command_msg(basic_connect, MBIM_MESSAGE_COMMAND_TYPE_QUERY, MBIM_CMD_BASIC_CONNECT_REGISTER_STATE, 0);
+	}
 
 	return mbim_send_command_msg();
 }