feat: Support multiple devices per driver crate (Fixes #90)#160
feat: Support multiple devices per driver crate (Fixes #90)#160ANAVHEOBA wants to merge 1 commit into
Conversation
- Updated Driver struct to support a list of 'devices' alongside the legacy 'meta' field. - Updated FullCrate generation to produce one entry per device/chip. - Updated read-driver-db to aggregate devices from both fields. - Regenerated driver-db-schema.json.
tdittr
left a comment
tdittr
left a comment
There was a problem hiding this comment.
Hey, thanks for taking the time to look into this. Unfortunately I don't think this solves the problem.
Drivers will be listed multiple times, once per device instead of once with an aggregated view of manufacturers, etc. I suspect this will also need changes in the frontend part to show the new data (e. g. multiple manufacturers).
Or as an intermediate we could aggregate the multiple Meta sections into one when we export it to the frontend.
I would also prefer to just updating the existing driver descriptions, rather than having legacy support for the meta section.
Also please don't forget to tick the box to accept that your contribution will be licensed under MIT.
| for meta in metas { | ||
| // We clone krate for each device because it is consumed by FullCrate::new | ||
| let full = match FullCrate::new(&driver, meta, krate.clone()) { | ||
| Ok(full) => full, | ||
| Err(e) => { | ||
| eprintln!("Error creating full crate: {e}"); | ||
| continue; | ||
| } | ||
| }; | ||
| output.push(full); | ||
| } |
There was a problem hiding this comment.
This adds one copy of the whole driver to the listing. So then we will have drivers listed multiple times if they support multiple devices. I think the aggregation of the devices needs to happen later, maybe even in the frontend?
| pub meta: Meta, | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub meta: Option<Meta>, | ||
| /// List of devices supported by this driver | ||
| #[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
| pub devices: Vec<Meta>, |
There was a problem hiding this comment.
I would rather see meta be replaced by devices. Also for the existing drivers. I guess category will need to be pulled out from meta as it is not really specific to each device.
| driver_db: &Driver, | ||
| meta: driver_db::Meta, |
There was a problem hiding this comment.
Why is meta being passed separately when it is already part of driver_db?
| dev_boards: driver_db.dev_boards.clone(), | ||
| interfaces: driver_db.interfaces.clone(), | ||
| resources: driver_db.resources.clone(), |
There was a problem hiding this comment.
Thanks for updating the schema, however it looks like you updated it by hand. Could you just run cargo run --release --bin read-driver-db it should update it automatically from the Rust types.
Sorry for not documenting this better!
2 participants
Checklist
driver-dbwhichget licensed under the CC0 license