First draft of the config structure for cmsis pack based flashing configuration

[Yatekii]

Ok, so I made a small draft of how I imagine hwo we could do this.
Please add your ideas and hints, I'll gladly incooperate them.

I suggest a config file structure like this:

$HOME/.config/probe-rs
    - repositories.yaml
    - repositories
        - foo
            - svd
                - chip_name.svd
            - algorithms
                - algorithm_name.yaml
            - targets
                - chip_name.yaml
            - .git

The chip_name.yaml contains a chip definition which links to an algorithm (by name).
It also contains a list of chip variants which all describe their memory layout.

The target can be selected with cargo-flash via cargo flash --target target_name::variant_name or cargo flash --target target_name which will select the first best variant it can find for the target.

To update the repositories we provide a command in cargo-flash too. Which will pull in the repository contents via the git URL provided in the repositories.yaml.

To not depend on a server, we ship a binary blob with a select set of sanitized configs, which will be placed in the probe.rs config dir at first run.

To extract the target descriptions we provide a tool too, which can parse the packs and generate the configs.

Maybe we should also not leave the algorithms separate as it just makes config management harder.
So we would just have the target configs.

The algorithm for target selection I imagine is:

• Parse all the target configs from the repositories.
• The ones with higher priorities overwrite the ones with lower priorities (I would take the order of the repository urls).
• The user can directly provide a config file via --config (or similar) which always wins in priority.
• The parsed targets are inserted into a list/hashmap from which the target then is selected by name.

I hope this covers it pretty well.

[jonas-schievink]

To not depend on a server, we ship a binary blob with a select set of sanitized configs, which will be placed in the probe.rs config dir at first run.

I would prefer not to do this as it clutters the user's home directory. Instead, we could fall back to the config data in the binary blob if no external config file defines a target.

Then we could provide a command line flag that will print the config file to stdout, so a user can still obtain the config, place it in the external directory, and edit it to override the defaults.

---

There's a lot of files that are missing the trailing newline. Does rustfmt not add that? I think we should enforce it to be present (it's good practice, many editors add it by default, and GitHub warns about it being missing).

---

Is it necessary that so many fields are pub? I'd prefer to provide getters to access them.

[Yatekii]

I would prefer not to do this as it clutters the user's home directory. Instead, we could fall back to the config data in the binary blob if no external config file defines a target.

Then we could provide a command line flag that will print the config file to stdout, so a user can still obtain the config, place it in the external directory, and edit it to override the defaults.

Ok, I can relate to this.
This implementation will be trickier tho and we have no possibility to update the targets other than to update the binary. Both might be a non factor tho.

There's a lot of files that are missing the trailing newline. Does rustfmt not add that? I think we should enforce it to be present (it's good practice, many editors add it by default, and GitHub warns about it being missing).

Sorry, I didn't run rustfmt yet as I made this as a draft for structure. And the basic idea. I will do this later. Also I don't understand why there needs to be a trailing newline even tho it's not an issue for me.

I personally dislike setters when it's just plain configs and the fields don't depend on each other/internal state. It's just boilerplate ...
Is there a particular reason you want this? So it is load only?

[jonas-schievink]

I personally dislike setters when it's just plain configs and the fields don't depend on each other/internal state. It's just boilerplate ...
Is there a particular reason you want this? So it is load only?

Yeah I guess it would be fine as-is... I just dislike adding so much API surface. Maybe using pub(crate) is the answer here; users of the crate shouldn't really need to access these types. Or we could make the entire module hierarchy private.

[Yatekii]

Maybe using pub(crate) is the answer here; users of the crate shouldn't really need to access these types. Or we could make the entire module hierarchy private.

+1 to that!

[Yatekii]

Hmm what if someone using probe-rs as a lib would like to add a config from codeside? Don't we want to enable that? Also, we might need a separate crate again to contain all the targets.

I would love to have it directly in probe-rs but I am not sure if a) this is nice design wise and b) is good to parse the config files properly in build.rs.

What do you think?

[jonas-schievink]

You could still have a public parser function that takes YAML data. Other than that, all targets should be in probe-rs and there should never really be the use case of having an external one. If a use case does come up, we can always change this later too.

[Yatekii]

Ok, I guess you are right! Then we also don't really need any repositories for now. This will make everything easier.

[jonas-schievink]

fyi, if you mention someone from a commit message, they'll get a notification when that commit is pushed anywhere. This isn't usually what you want.

[Yatekii]

Oh, I am very sorry for the notifications. I will leave the @ next time :)

I also added tests and indeed the intersects_range() method was broken. Good spot.

[Yatekii]

all targets should be in probe-rs

This is gonna be tricky. Because either we need manual parsing (parse the yaml into a HashMap tree structure or the likes) or we need the struct already present, which is not the case as we are doing this (Build.rs) before compilation which means the struct is not yet known.

[Yatekii]

Ok so the new configs are mostly fixed now.
I need to test it tomorrow (or when I find time) but other than that it should be good to go with any pack config now :)