Conversation
This was recommended by an AI. As an inexperienced Rust programmer, I had drivers reply to settings through a boxed closure. What was I thinking? The AI (ChatGPT 5 in GitHub's Copilot) recommended using a type instead. This allows me to define methods that are simpler to use, avoids an extra heap allocation for every incoming setting, and define the methods to consume the type so you can only reply once.
The AI also recommended using `async` methods instead of forcing a boxed, pinned future -- like I specified. This should reduce heap allocations. Ideally the `Driver` trait should be using the async notation but I couldn't figure out a way to add the `Send` requirement to the Future that's returned.
Define `TryFrom` traits for strings and string slices. Update some methods so they accept types that can be converted. This simplifies the API for driver writers in that they don't have to do the conversions. This commit was recommended by an AI, although I did the modifications.
I was including `serde_derive`, but that automatically gets included when you enable the "derive" feature on `serde`.
Rather than have handwritten methods to validate and obtain parameters from the driver config map, the AI recommended using the `serde` crate. This large commit redefines `DriverConfig` to be a wrapping structure that adds useful conversions into a driver's specific needs.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
These API improvements were the recommendation of ChatGPT in GitHub's Copilot feature. Some of the code it flagged was stuff I wrote earlier when I was less experienced with Rust. They were very good changes. Although ChatGPT showed examples of how the improvements would look and be used, I wrote all the changes required in the
drmem-apiand driver crates.The first change converted a boxed closure into an enumerated type which required no allocating.
The second was to use
asyncmethods in the driver crate instead of specifying complicated, boxed, pinned futures with specialty traits.Next, functions and methods that needed any path of a device name were requiring the caller to create the correct type. Now we've defined an Into<> trait so callers can pass
&strand the functions will convert it internally. This really cleaned up some code.Finally, instead of hand-writing code to verify and access the driver config parameters, we now use
serde. A driver can describe their parameters in a struct andserdewill do the validation. This is a huge usability win for authors of device drivers.