Skip to content

Add read and write functions - #4

Closed
C0rn3j wants to merge 1 commit into
vpelletier:masterfrom
C0rn3j:patch-1
Closed

Add read and write functions#4
C0rn3j wants to merge 1 commit into
vpelletier:masterfrom
C0rn3j:patch-1

Conversation

@C0rn3j

@C0rn3j C0rn3j commented Sep 18, 2024

Copy link
Copy Markdown

Hello there, I am currently plumbing sc-controller which has bundled some libraries included this one, into which it added these two helper functions.
I intend to tear it out and make it a proper dependency.

Would it make sense to have them here in the upstream library to you, or should I figure out how to override the library with them instead?

What the write() is used for can be found here for example, in the commit that introduced it - C0rn3j/sc-controller@e550828

@vpelletier

Copy link
Copy Markdown
Owner

The intention in this code is that the HIDRaw class is used for hidraw-specific features, and that anything related to the plain file itself (open, read, write, close, controlling whether it is blocking, ...) should be done on the device that was provided to its constructor and not on instances of this class. If I go down the path of exposing these 2 methods, then why not all file methods (fileno, readinto, ...) and attributes ? Which would mean either a lot of boilerplate code to define each (and then someone will be unsatisfied because their custom extension of the file object api is not covered) or __{set,get,del}attr__ methods to pass accesses through thereby smashing both namespaces into one and losing the ability to extend the API of HIDRaw itself.

Also, note that this class accepts an integer as device (file descriptor), which would cause the added methods (as they are implemented in the current patch) to raise. My impression is that covering this case would introduce too much avoidable complexity - avoidable by having the caller code keep track of the underlying device.

So IMHO the cleanest solution is that you do not only keep track of the HIDRaw instance in DS5HidRawController, but separately also the underlying file. Does this make sense ?

@C0rn3j

C0rn3j commented Jul 20, 2026

Copy link
Copy Markdown
Author

Does this make sense ?

I suppose what you meant was changing it like so:

From:

	hidraw = HIDRaw(open(filename, "w+b"))
	c = TestSC(driver, None, hidraw)
...
		self._hidrawdev: HIDRaw = hidrawdev
...
			data = self._hidrawdev.read(self._packet_size)

To:

	device_file = open(filename, "w+b")
	hidraw = HIDRaw(device_file)
	c = TestSC(driver, None, hidraw, device_file)
...
		self._hidrawdev: HIDRaw = hidrawdev
		self._device_file: BinaryIO = device_file
...
			data = self._device_file.read(self._packet_size)

Which I've now done - I suppose I'll send a PR with typing additions soon so I can tear out the vendored in copy of this project.

@C0rn3j C0rn3j closed this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants