-
Notifications
You must be signed in to change notification settings - Fork 180
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
refactor(api): module live data type #17234
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, though I'm confused why we would return empty/unknown module data if the validator doesn't pass and not log and ignore it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah this will simplify validating the live data
I'm not sure why either, but I didn't want to change the behavior too much in this PR so i kept it like this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refactor looks good, definitely makes more sense to structure this this way instead of the open returned data structures we had before.
Overview
This PR defines each module's live data into more specific
TypedDict
s. Doing so allows us to (1) better visualize what data are expected from the module hardware, (2) simplify the_add_module_substate
function in the protocol engine module state store. This is particular important when we are about to add a new module.I also added
from_live_data
class generator method to some of the substates (as this is the only time module live data is used). Any concerns regarding pushing this logic down to the individual substate?