added ObexListenerContext(Stream rs) for Android file transfer #356
Conversation
added ContentMd5 ObexHeader
| @@ -1038,8 +1050,16 @@ public override WebResponse GetResponse() | |||
| ObexStatusCode status; | |||
| MemoryStream ms = new MemoryStream(); | |||
| WebHeaderCollection responseHeaders = new WebHeaderCollection(); | |||
| // Added David Rodgers to facilitate sending an Md5 checksum in the header | |||
| // could be expanded to loop through Headers and add any headers that are set by the user | |||
| if (Headers["ContentMd5"] != null) | |||
There was a problem hiding this comment.
Do you need to add the existing request header here? - because the responseHeaders should contain the headers as sent by the remote device along with the retrieved file.
There was a problem hiding this comment.
When sending the file I set the user defined header request.Headers.Add("ContentMd5", md5); ("ContentMd5" would change to "User0" with your new changes) before calling request.GetResponse(); The call to GetResponse() then creates responseHeaders as a new WebHeaderCollection() instance effectively wiping out the previously set user defined header. Other headers are added in ObexWebRequest:DoPut(); With your new user defined headers where would we set them when sending a file?
There was a problem hiding this comment.
I will wait until I find out how to set a user defined header so I can implement and test before resubmitting.
There was a problem hiding this comment.
You have not commented about public ObexListenerContext(Stream rs) with a stream parameter instead of socket, what are your thoughts about it?
There was a problem hiding this comment.
Sorry I've been distracted and forgot to follow up on this. Now that the listener can work from either a stream or a socket, I don't think the public constructor is needed because this can be created through the normal listener flow.
There was a problem hiding this comment.
I can see where you test for socket == null in ObexListenerContext at line 257 and assign stream on line 259 but this is not used. Line 283 says "// call to stream implementation to go here" so there is no ObexListenerContext returned. So this does not seem to be any further ahead with regard to getting the ObexListenerContext "through the normal listener flow". My other issue is that I have to use BluetoothListener rather than ObexListener because I need to set a custom service GUID. It would be great if we could pass a custom GUID to the BluetoothListener constructor. This is why I created the public ObexListenerContext constructor which took a stream argument. I want to use ObexListenerContext but also need a custom service GUID.
There was a problem hiding this comment.
Good point, then it should expose the public constructor for those cases. There is definitely scope for more flexibility in the listener so it can support custom services
There was a problem hiding this comment.
Will you be making changes to support custom services and when would you expect to include them?
There was a problem hiding this comment.
I think based on time available to this at the moment we assume that it won't happen soon. It'll possibly require work to both the BluetoothListener and ObexListener to fully support.
|
No problem Peter, I only get to look at it occasionally myself.
I will test to make sure I can achieve what we require with your changes. Are they changes included on nuget.org yet or should I use the code from main on Git?
Thanks for all your help.
Regards,
David
From: Peter Foot ***@***.***>
Sent: Saturday, November 25, 2023 4:50 AM
To: inthehand/32feet ***@***.***>
Cc: David Rodgers ***@***.***>; Author ***@***.***>
Subject: Re: [inthehand/32feet] added ObexListenerContext(Stream rs) for Android file transfer (PR #356)
@peterfoot commented on this pull request.
________________________________
In InTheHand.Net.Obex/ObexWebRequest.cs<#356 (comment)>:
@@ -1038,8 +1050,16 @@ public override WebResponse GetResponse()
ObexStatusCode status;
MemoryStream ms = new MemoryStream();
WebHeaderCollection responseHeaders = new WebHeaderCollection();
+ // Added David Rodgers to facilitate sending an Md5 checksum in the header
+ // could be expanded to loop through Headers and add any headers that are set by the user
+ if (Headers["ContentMd5"] != null)
Sorry I've been distracted and forgot to follow up on this. Now that the listener can work from either a stream or a socket, I don't think the public constructor is needed because this can be created through the normal listener flow.
—
Reply to this email directly, view it on GitHub<#356 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AY6BX3NQJJRXO2Q4FDSUFFDYGDT57AVCNFSM6AAAAAA6QEFR5CVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBYGMZTKNZYGY>.
You are receiving this because you authored the thread.Message ID: ***@***.******@***.***>>
|
Added ObexListenerContext(Stream rs) for file transfer between Windows and Android due to System.Net.Sockets not being compatible with Android sockets.
added ContentMd5 ObexHeader to facilitate sending Md5 checksum to destination with file.