-
Notifications
You must be signed in to change notification settings - Fork 2
Open PROT changes to pldm-lib. #2
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
base: main
Are you sure you want to change the base?
Conversation
…ceholder member functions. IPC calls will be placed into their bodies and fd_internal as well as fd_ops content will be moved to a fd Hubris driver task.
| #[derive(Debug, Copy, Clone, PartialEq)] | ||
| #[repr(C)] | ||
| #[derive(FromBytes, IntoBytes)] | ||
| //#[repr(C)] |
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.
I haven't checked in depth what triggered this change.
Is the layout of this type critical for (de)serialization?
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 is a good question. I can't say why this changed was made. It may have been a co-pilot adjustment. I'll change it back.
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.
Is this inteded as another package?
Then this belongs in its on directory with its own /src/lib.rs.
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 should be removed completely if not needed.
If some timer interface is needed, maybe use a dedicated crate for that.
(for example embedded-timers)
| [workspace] | ||
| members = [ | ||
| "pldm-common" | ||
| ] |
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.
Moving pldm-lib into its own directory and adding it to the workspace members might be cleaner and probably better for packaging releases.
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.
I have thought about that, but I want to keep pldm-common as part of pldm-lib. Don't we want pldm-common to exist within pldm-lib?
Async executor removed. Firmware device api, fd_ops and fd_internal, will be in a Hubris driver task.