Request Size Management and Misc. Housekeeping #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was always the odd man out of the bunch. Changing to make it fit better with ByteMap and BitMap This isn't user-facing and there are no functional changes in this commit, just improving the typename's conformity.
A bit of Photoshop to remove the Arduino IDE version number (1.8.5) and change the update fail condition "reconnect()" function to "connect()", as "reconnect()" is no longer part of the library.
Missed it by *that* much. Should have been in 65a757b. skip ci
In my haste I forgot to check these when I merged the pull request. skip ci
The request size should be shared by all controller objects using the same logical "port". Only one controller can be active at a time with any given shared data group, so it doesn't make sense to keep this exclusive to the instance. This also means that when multiple controller types are used with shared data, changes in the request size will also transfer to the generic ExtensionPort object. Important for specificInit usage when using multiple types.
Finally deciding to pull the trigger on this. Previously, requestSize was completely up to the user and wasn't managed by the library at all because all controllers support the 6-byte request mode. The existence of the Classic Controller "high res" mode creates a conflict, because connecting to the controller in that mode requires an 8 byte request size which then persists after disconnecting even for controllers that don't make use of it. So I'm changing the idiom. The *library* now manages request size if it needs to. If the user wants to change the request size, they can do that after connection. This makes the library more efficient and flexible. Plus even user-set request sizes should be different for each controller type, which would be set after the controller is initialized (read: identified) anyways.
The only difference between these two was that the 'reset' function also put the request size back to minimum. Now that the request size is reset on each connection attempt these two functions are one and the same.
The larger request size now needs to be set for each connection attempt. This was also broken by #58, which removed the initial update in the connect() function. That data was used for the "third party" check, so an update needs to be performed immediately after connection. Whoops.
This gives a slight efficiency improvement (vs clearing data every time) and also preserves the current control data until a new controller has connected to replace it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The primary feature of this PR is changing the idiom surrounding
requestSize.Previously,
requestSizewas handled exclusively by the user. The controller object was initialized using the smallest valid request size for control data (6 bytes) and the user had the option to increase it as needed. This was exclusive to each object, so in a program with multiple mixed controller objects the request size was saved and could potentially be unique for each one.This PR changes that approach.
requestSizeis a fundamental part of the communication flow and is now both:connect()).This has two advantages. First, in programs using a shared "port" object (i.e multiple controller types), modifying the
requestSizein one object will affect the others. This means that a controller type that requires 8 bytes for its updates (the NES Mini controller) can change therequestSizefor the generic "port" object, which will then successfully request 8 bytes for its control updates. Second, if the request size is increased, this is reset on connection so that other controller types that don't use a larger request size won't be bogged down by the extra requests on the I²C bus.This PR also fixes NES controller support, which was broken in the examples since #58 because the
connect()function no longer populates the first data read.Also included in this PR are some miscellaneous housekeeping tweaks:
CtrlIndexasIndexMapfor better conformity with theByteMapandBitMaptypenames.