[9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[Romano]

Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
---
diff c9c5d6455a6502ffb0f5490d5602396852261f4c 755ac2c11ce134f76d83a2a42edb07a6304d1de8
--- a/sys/man/4/nusb
+++ b/sys/man/4/nusb
@@ -194,6 +194,10 @@
 The
 .B ctl
 file supplies the device geometry and partitions when read.
+.I Ptp
+configures and exposes a camera's (and often a mobile
+device's) storage to allow transferring data via the
+Picture Transfer Protocol (PTP).
 .SS Ethernet
 .I Ether
 handles USB ethernet devices. The file system provided is
--- a/sys/src/cmd/nusb/ptp/ptp.c
+++ b/sys/src/cmd/nusb/ptp/ptp.c
@@ -75,9 +75,8 @@
 
 static int debug = 0;
 static ulong time0;
-static int maxpacket = 64;
-static int sessionId = 1;
-static int transId = 1;
+static int sessionId;
+static int transId;
 
 static Node **nodes;
 static int nnodes;
@@ -556,7 +555,7 @@
 		}
 
 		/*
-		 * another proc migh'v come in and done it for us,
+		 * another proc might've come in and done it for us,
 		 * so check the cache again.
 		 */
 		if(y = cachednode(path, &f))
@@ -1048,6 +1047,7 @@
 	sendp(iochan, ioproc());
 
 	sessionId = getpid();
+	transId   = 0;
 	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
 		sysfatal("open session: %r");
 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[unobe]

The transaction id is meant to be 0, not 1, per the spec (and what the
patch shows): slipped up on the first line of the commit message.

Quoth Romano <unobe@cpan.org>:
> 
> Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
> ---
> diff c9c5d6455a6502ffb0f5490d5602396852261f4c 755ac2c11ce134f76d83a2a42edb07a6304d1de8
> --- a/sys/man/4/nusb
> +++ b/sys/man/4/nusb
> @@ -194,6 +194,10 @@
>  The
>  .B ctl
>  file supplies the device geometry and partitions when read.
> +.I Ptp
> +configures and exposes a camera's (and often a mobile
> +device's) storage to allow transferring data via the
> +Picture Transfer Protocol (PTP).
>  .SS Ethernet
>  .I Ether
>  handles USB ethernet devices. The file system provided is
> --- a/sys/src/cmd/nusb/ptp/ptp.c
> +++ b/sys/src/cmd/nusb/ptp/ptp.c
> @@ -75,9 +75,8 @@
>  
>  static int debug = 0;
>  static ulong time0;
> -static int maxpacket = 64;
> -static int sessionId = 1;
> -static int transId = 1;
> +static int sessionId;
> +static int transId;
>  
>  static Node **nodes;
>  static int nnodes;
> @@ -556,7 +555,7 @@
>  		}
>  
>  		/*
> -		 * another proc migh'v come in and done it for us,
> +		 * another proc might've come in and done it for us,
>  		 * so check the cache again.
>  		 */
>  		if(y = cachednode(path, &f))
> @@ -1048,6 +1047,7 @@
>  	sendp(iochan, ioproc());
>  
>  	sessionId = getpid();
> +	transId   = 0;
>  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
>  		sysfatal("open session: %r");
>  
> 
> 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 0, not 1.

[unobe]

Ping.

Re: transaction ID, it must be set to 0 for opening a session, not 1.
See section 2.3 (page 3) of this pdf:
https://people.ece.cornell.edu/land/courses/ece4760/FinalProjects/f2012/jmv87/site/files/PTP%20Protocol.pdf

And also (I suppose more importantly, the standard referenced) section
9.3.1 (page 50) of this pdf:
https://people.ece.cornell.edu/land/courses/ece4760/FinalProjects/f2012/jmv87/site/files/pima15740-2000.pdf

Quoth unobe@cpan.org:
> The transaction id is meant to be 0, not 1, per the spec (and what the
> patch shows): slipped up on the first line of the commit message.
> 
> Quoth Romano <unobe@cpan.org>:
> > 
> > Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
> > ---
> > diff c9c5d6455a6502ffb0f5490d5602396852261f4c 755ac2c11ce134f76d83a2a42edb07a6304d1de8
> > --- a/sys/man/4/nusb
> > +++ b/sys/man/4/nusb
> > @@ -194,6 +194,10 @@
> >  The
> >  .B ctl
> >  file supplies the device geometry and partitions when read.
> > +.I Ptp
> > +configures and exposes a camera's (and often a mobile
> > +device's) storage to allow transferring data via the
> > +Picture Transfer Protocol (PTP).
> >  .SS Ethernet
> >  .I Ether
> >  handles USB ethernet devices. The file system provided is
> > --- a/sys/src/cmd/nusb/ptp/ptp.c
> > +++ b/sys/src/cmd/nusb/ptp/ptp.c
> > @@ -75,9 +75,8 @@
> >  
> >  static int debug = 0;
> >  static ulong time0;
> > -static int maxpacket = 64;
> > -static int sessionId = 1;
> > -static int transId = 1;
> > +static int sessionId;
> > +static int transId;
> >  
> >  static Node **nodes;
> >  static int nnodes;
> > @@ -556,7 +555,7 @@
> >  		}
> >  
> >  		/*
> > -		 * another proc migh'v come in and done it for us,
> > +		 * another proc might've come in and done it for us,
> >  		 * so check the cache again.
> >  		 */
> >  		if(y = cachednode(path, &f))
> > @@ -1048,6 +1047,7 @@
> >  	sendp(iochan, ioproc());
> >  
> >  	sessionId = getpid();
> > +	transId   = 0;
> >  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
> >  		sysfatal("open session: %r");
> >  
> > 
> > 
> 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[ori]

Looks reasonable; is there any reason you needed to change the session
id here too?

Quoth Romano <unobe@cpan.org>:
> 
> Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
> ---
> diff c9c5d6455a6502ffb0f5490d5602396852261f4c 755ac2c11ce134f76d83a2a42edb07a6304d1de8
> --- a/sys/man/4/nusb
> +++ b/sys/man/4/nusb
> @@ -194,6 +194,10 @@
>  The
>  .B ctl
>  file supplies the device geometry and partitions when read.
> +.I Ptp
> +configures and exposes a camera's (and often a mobile
> +device's) storage to allow transferring data via the
> +Picture Transfer Protocol (PTP).
>  .SS Ethernet
>  .I Ether
>  handles USB ethernet devices. The file system provided is
> --- a/sys/src/cmd/nusb/ptp/ptp.c
> +++ b/sys/src/cmd/nusb/ptp/ptp.c
> @@ -75,9 +75,8 @@
>  
>  static int debug = 0;
>  static ulong time0;
> -static int maxpacket = 64;
> -static int sessionId = 1;
> -static int transId = 1;
> +static int sessionId;
> +static int transId;
>  
>  static Node **nodes;
>  static int nnodes;
> @@ -556,7 +555,7 @@
>  		}
>  
>  		/*
> -		 * another proc migh'v come in and done it for us,
> +		 * another proc might've come in and done it for us,
>  		 * so check the cache again.
>  		 */
>  		if(y = cachednode(path, &f))
> @@ -1048,6 +1047,7 @@
>  	sendp(iochan, ioproc());
>  
>  	sessionId = getpid();
> +	transId   = 0;
>  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
>  		sysfatal("open session: %r");
>  
> 
> 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[unobe]

Quoth ori@eigenstate.org:
> Looks reasonable; is there any reason you needed to change the session
> id here too?

I just removed its initialization, since it's overwritten anyway to
the pid.  (Setting to 1 at the top superfluous.)

> 
> Quoth Romano <unobe@cpan.org>:
> > 
> > Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
> > ---
> > diff c9c5d6455a6502ffb0f5490d5602396852261f4c 755ac2c11ce134f76d83a2a42edb07a6304d1de8
> > --- a/sys/man/4/nusb
> > +++ b/sys/man/4/nusb
> > @@ -194,6 +194,10 @@
> >  The
> >  .B ctl
> >  file supplies the device geometry and partitions when read.
> > +.I Ptp
> > +configures and exposes a camera's (and often a mobile
> > +device's) storage to allow transferring data via the
> > +Picture Transfer Protocol (PTP).
> >  .SS Ethernet
> >  .I Ether
> >  handles USB ethernet devices. The file system provided is
> > --- a/sys/src/cmd/nusb/ptp/ptp.c
> > +++ b/sys/src/cmd/nusb/ptp/ptp.c
> > @@ -75,9 +75,8 @@
> >  
> >  static int debug = 0;
> >  static ulong time0;
> > -static int maxpacket = 64;
> > -static int sessionId = 1;
> > -static int transId = 1;
> > +static int sessionId;
> > +static int transId;
> >  
> >  static Node **nodes;
> >  static int nnodes;
> > @@ -556,7 +555,7 @@
> >  		}
> >  
> >  		/*
> > -		 * another proc migh'v come in and done it for us,
> > +		 * another proc might've come in and done it for us,
> >  		 * so check the cache again.
> >  		 */
> >  		if(y = cachednode(path, &f))
> > @@ -1048,6 +1047,7 @@
> >  	sendp(iochan, ioproc());
> >  
> >  	sessionId = getpid();
> > +	transId   = 0;
> >  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
> >  		sysfatal("open session: %r");
> >  
> > 
> > 
> 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[ori]

Wait, I retract that.

Quoth Romano <unobe@cpan.org>:
> 
> Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
>  	sessionId = getpid();
> +	transId   = 0;
>  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)

Setting transid=0 here means that when we
enter vptprpc, we do:

	t = transId++;
	...
	PUT4(rpc.transid, t);

which will put *0* into the transaction ID.
Am I missing something here?

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[unobe]

So this patch looks good as well?  I don't have a commit bit.

Quoth ori@eigenstate.org:
> Wait, I retract that.
> 
> Quoth Romano <unobe@cpan.org>:
> > 
> > Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
> >  	sessionId = getpid();
> > +	transId   = 0;
> >  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
> 
> Setting transid=0 here means that when we
> enter vptprpc, we do:
> 
> 	t = transId++;
> 	...
> 	PUT4(rpc.transid, t);
> 
> which will put *0* into the transaction ID.
> Am I missing something here?
> 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[igor]

> So this patch looks good as well?  […]

Thanks!

Just tested it and can now browse iPhone photos on 9front ☺

After applying the fix and plugging in an iPhone, photos are
accessible via '/shr':

  term% ls '/shr/sdU96cb0/Internal Storage/DCIM'
  […]
  '/shr/sdU96cb0/Internal Storage/DCIM/202312__'
  '/shr/sdU96cb0/Internal Storage/DCIM/202401__'

Unless there are any more objections or someone beats me to it I will
merge it this weekend.

Cheers,
Igor

Quoth unobe@cpan.org:
> So this patch looks good as well?  I don't have a commit bit.
> 
> Quoth ori@eigenstate.org:
> > Wait, I retract that.
> > 
> > Quoth Romano <unobe@cpan.org>:
> > > 
> > > Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
> > >  	sessionId = getpid();
> > > +	transId   = 0;
> > >  	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
> > 
> > Setting transid=0 here means that when we
> > enter vptprpc, we do:
> > 
> > 	t = transId++;
> > 	...
> > 	PUT4(rpc.transid, t);
> > 
> > which will put *0* into the transaction ID.
> > Am I missing something here?
> > 
> 

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 1, not 0.

[ori]

Quoth unobe@cpan.org:
> 
> So this patch looks good as well?  I don't have a commit bit.

No. It doesn't seem to do what you said it does.

I thought the goal was to start from transaction id 1,
but your change makes it start from id 0.

Re: [9front] [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 0, not 1.

[unobe]

Quoth ori@eigenstate.org:
> Quoth unobe@cpan.org:
> > 
> > So this patch looks good as well?  I don't have a commit bit.
> 
> No. It doesn't seem to do what you said it does.
> 
> I thought the goal was to start from transaction id 1,
> but your change makes it start from id 0.
> 

This is what I wrote a month ago:

Quoth unobe@cpan.org:
> Ping.
> 
> Re: transaction ID, it must be set to 0 for opening a session, not 1.
> See section 2.3 (page 3) of this pdf:
> https://people.ece.cornell.edu/land/courses/ece4760/FinalProjects/f2012/jmv87/site/files/PTP%20Protocol.pdf
> 
> And also (I suppose more importantly, the standard referenced) section
> 9.3.1 (page 50) of this pdf:
> https://people.ece.cornell.edu/land/courses/ece4760/FinalProjects/f2012/jmv87/site/files/pima15740-2000.pdf
> 
> Quoth unobe@cpan.org:
> > The transaction id is meant to be 0, not 1, per the spec (and what the
> > patch shows): slipped up on the first line of the commit message.
> > 

I've changed the subject line (the first line of the commit message)
in this reply and here's my original message (with the corrected
subject line) from September 27:

From: Romano <unobe@cpan.org>
Date: Wed Sep 27 23:58:39 -0700 2023
Subject: [PATCH 3/6] nusb/ptp: add mention to nusb man page and fix implementation per spec to start with transaction id 0, not 1.


Apple iPhones can now be used to transfer images. There is still an issue with the iPhone permission, wherein an actor has to be pretty quick on the trigger to Allow the connection from the iPhone side, but that would require a pause somewhere in the handshake that I did not have time to look into further. This patch at least allows the connection to succeed when permitted.
---
diff c9c5d6455a6502ffb0f5490d5602396852261f4c 755ac2c11ce134f76d83a2a42edb07a6304d1de8
--- a/sys/man/4/nusb
+++ b/sys/man/4/nusb
@@ -194,6 +194,10 @@
 The
 .B ctl
 file supplies the device geometry and partitions when read.
+.I Ptp
+configures and exposes a camera's (and often a mobile
+device's) storage to allow transferring data via the
+Picture Transfer Protocol (PTP).
 .SS Ethernet
 .I Ether
 handles USB ethernet devices. The file system provided is
--- a/sys/src/cmd/nusb/ptp/ptp.c
+++ b/sys/src/cmd/nusb/ptp/ptp.c
@@ -75,9 +75,8 @@
 
 static int debug = 0;
 static ulong time0;
-static int maxpacket = 64;
-static int sessionId = 1;
-static int transId = 1;
+static int sessionId;
+static int transId;
 
 static Node **nodes;
 static int nnodes;
@@ -556,7 +555,7 @@
 		}
 
 		/*
-		 * another proc migh'v come in and done it for us,
+		 * another proc might've come in and done it for us,
 		 * so check the cache again.
 		 */
 		if(y = cachednode(path, &f))
@@ -1048,6 +1047,7 @@
 	sendp(iochan, ioproc());
 
 	sessionId = getpid();
+	transId   = 0;
 	if(ptprpc(nil, OpenSession, 1, sessionId) < 0)
 		sysfatal("open session: %r");