Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Somehow return actual number of bytes read? #85

Closed
ejluttmann opened this issue Jul 23, 2019 · 8 comments

Comments

@ejluttmann
Copy link

commented Jul 23, 2019

When calling ReadAsync(), it would be nice to know how many bytes were actually read. In some cases, especially with Bulk transfers, its possible to receive less bytes than the requested number of bytes from the USB device based on the protocol being used. Would it would be possible to add an optional parameter that would return the number of bytes actually read? Right now it looks to just be zero filled.

@MelbourneDeveloper

This comment has been minimized.

Copy link
Owner

commented Jul 24, 2019

Good call on this.

Should the array be truncated to the size returned?

Should the result be a list and that list be truncated?

Should the result be a struct with the count?

For maintainability I would say truncate, but for performance struct. Generally I prefer maintainability over performance...

Thoughts?

@ejluttmann

This comment has been minimized.

Copy link
Author

commented Jul 24, 2019

My initial thought was truncating the size of the byte array returned, but for larger data transfers that could be a significant performance hit. Really, just need the ability to obtain the number of actual bytes read - this could just be an 'int' defined in IDevice. Same thing could be added for WriteAsync(), to get the actual number of bytes written (although probably not as necessary).

@MelbourneDeveloper

This comment has been minimized.

Copy link
Owner

commented Jul 29, 2019

@ejluttmann please have a look at this branch and tell me what you think:

develop...#85-ReadResult

@MelbourneDeveloper

This comment has been minimized.

Copy link
Owner

commented Jul 29, 2019

@ejluttmann I've only really solved the problem in Windows, but I've refactored so that it can be done for any platform.

@MelbourneDeveloper MelbourneDeveloper added this to Todo in 3.0 Jul 29, 2019

@MelbourneDeveloper

This comment has been minimized.

Copy link
Owner

commented Jul 29, 2019

@ejluttmann this is the main change for Windows:

image

@ejluttmann

This comment has been minimized.

Copy link
Author

commented Jul 29, 2019

That looks like it would work.

@MelbourneDeveloper

This comment has been minimized.

Copy link
Owner

commented Jul 30, 2019

@ejluttmann OK. It's in the Develop branch now. It will go in to 3.0 Beta 2

@MelbourneDeveloper MelbourneDeveloper moved this from Todo to Done in 3.0 Jul 30, 2019

@MelbourneDeveloper

This comment has been minimized.

Copy link
Owner

commented Aug 3, 2019

@ejluttmann this has been included in V3.0 Beta 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.