Add file config support for ravedude

[Creative0708]

Followup PR to #521

What the title says. This adds a board named custom to ravedude that reads from Ravedude.toml from the project directory.

Works with this config (in Ravedude.toml):

name = "ATtiny85 through AVRISP"

[reset]
automatic = true

[avrdude]
programmer = "avrisp"
partno = "t85"
baudrate = 19200
do_chip_erase = true

And this cargo runner:

runner = "/path/to/ravedude custom -P /dev/ttyUSB0"

Minor detail but this PR also changes get_board to return anyhow::Result<Box<dyn Board>>, if that matters.

Anyways, format and implementation of custom board support in ravedude is up for discussion. If we reach a consensus about what would be best, this PR will be marked as ready for review.

[Creative0708]

Also, some further notes:

• Leveraging the power of serde and the toml crate, we get nice error messages whenever a field is missing:

  
  # [...]

[avrdude] programmer = "avrisp"

partno = "t85"

baudrate = 19200 do_chip_erase = true

Produces

Error: TOML parse error at line 7, column 1 | 7 | [avrdude] | ^^^^^^^^^ missing field partno

- "Dumping" a built-in board configuration to toml is not yet done. Due to how the `Board` trait is implemented, it's impossible to get the board ports from a `dyn Board` or the like. Most likely, we'll just transcribe the source of `board.rs` into several `BoardConfig`s.

- This is fully backwards-compatible with previous ravedude configurations, as it just adds a board named "custom". Now that I've got some chance to think, this feels a bit... clunky.
  I'd rather have a mandatory Ravedude.toml in the project root for all boards. My reasoning for this is that a TOML file with all the keys already present would be more intuitive/familiar and less confusing than modifying the runner in `.cargo/config.toml` with flag options only discoverable by running `ravedude -h`.
  Additionally, if someone wants to modify their board setup, they'll have to generate the config for their board, _then_ modify it. This is an extra step in what I believe should be an easy modification of... something.
  While breaking every past user's project in a new version is off the table, maybe we could display a warning like ```Using a board argument is deprecated. Run `ravedude --dump-toml uno > Ravedude.toml` to generate a new config, then change the runner to `ravedude -P /dev/ttyUSB0`.``` We could then change cargo-generate to use Ravedude.toml configured with the correct board in the setup process.
  Obviously, this is my personal opinion. This is up for discussion of course.

[Rahix]

Leveraging the power of serde and the toml crate, we get nice error messages whenever a field is missing:

nice! :+1:

This is fully backwards-compatible with previous ravedude configurations, as it just adds a board named "custom". Now that I've got some chance to think, this feels a bit... clunky.

Feel free to drop the entire Board trait abstraction. I was assuming that you were going to do that anyway :D I see no reason to stick to the existing architecture here - it was mostly written this way to quickly get something running.

Whatever we end up doing with the "builtin boards", they should also simply get encoded in the new TOML format somewhere, as you suggested.

I'd rather have a mandatory Ravedude.toml in the project root for all boards.

Interesting idea. My original design approach was to model ravedude after probe-run. A simple tool to be dropped into the cargo config as a project runner.

Your suggestion reminds me of the approach of cargo-embed which also has a more or less mandatory config file for all its parameters.

I don't see a big reason why not to go down this route, although I think we should consider the specifics carefully:

• First of all, I would then suggest to drop a lot of the commandline arguments and push them into Ravedude.toml instead. Something like -P should stay, but also get a corresponding setting in the file.

• We suggest building new projects using the avr-hal-template which could also generate an initial Ravedude.toml without problems.

• I'm not a fan of the suggestion that people will always encode all board settings in their own Ravedude.toml rather than referencing a "well known board". The reason is that this prevents us from rolling out fixes/changes for the well known boards. Unless customization is necessary, I think users should still just reference a board by its name.

• A possible approach would maybe be one where a board may be referenced by name, but whether it is or isn't, custom settings can be used to override or define parameters.

To put it into concrete examples:

# Ravedude.toml for using well known board
[general]
console = true
baudrate = 57600

board = "uno"

# Ravedude.toml for using a completely custom board
[general]
console = true
baudrate = 57600

[board]
name = "ATtiny85 through AVRISP"

[board.reset]
automatic = true

[board.avrdude]
programmer = "avrisp"
partno = "t85"
baudrate = 19200
do_chip_erase = true

# Ravedude.toml for customizing well known board
[general]
console = true
baudrate = 57600

[board]
name = "Uno without automatic reset"
inherit = "uno"

[board.reset]
automatic = false

What do you think?

[Creative0708]

Feel free to drop the entire Board trait abstraction. I was assuming that you were going to do that anyway :D

Whatever we end up doing with the "builtin boards", they should also simply get encoded in the new TOML format somewhere, as you suggested.

Sounds good!

• First of all, I would then suggest to drop a lot of the commandline arguments and push them into Ravedude.toml instead. Something like -P should stay, but also get a corresponding setting in the file.

I assume in this case command-line options would override the TOML file. Would we want a warning to be printed in that case?

I don't see a big reason why not to go down this route,

Does this signify approval? Because I'd rather not change things just for the sake of changing things. If mandatory TOML configs don't benefit the project in any way, they shouldn't be implemented.

• I'm not a fan of the suggestion that people will always encode all board settings in their own Ravedude.toml rather than referencing a "well known board". The reason is that this prevents us from rolling out fixes/changes for the well known boards. Unless customization is necessary, I think users should still just reference a board by its name.

This is a very good point, and I hadn't considered that. Yeah, I agree that this would be a better solution.

The examples look great! However, I would like a consistent case across keys. (Either snake_case or kebab-case is fine, although I'm leaning towards kebab case to mirror Cargo.toml.)

Also, annoyingly, serde doesn't have "inheritance" behavior built-in, and from some very basic searching around the web I've gathered that the best way to do this is just to make everything an Option<whatever> and merge the configs after deserialization. Not such a big deal though.

A final note/question: Would this be considered a breaking change...? If not, we'd need to find a way to keep this backwards-compatible. Maybe a "default" config that's used if the file doesn't exist? It depends on how much of a priority backwards compatibility is.

[Rahix]

Hi, sorry for only getting back to you now...

A final note/question: Would this be considered a breaking change...? If not, we'd need to find a way to keep this backwards-compatible. Maybe a "default" config that's used if the file doesn't exist? It depends on how much of a priority backwards compatibility is.

To me, it's really important not to completely break existing users. At least, there should be a trivial migration path that doesn't require deeper understanding of the inner workings of ravedude and avrdude...

For simplicity, my suggestion would be to keep the existing invocations working as is, as long as no Ravedude.toml is present. In such a case, I'd like a warning to inform users that they should migrate.

If all else fails, a hacky solution to do this might be to throw the current "legacy" argument parser into a separate module and invoke it when no Ravedude.toml is found. Then the given options are used to construct an in-memory "compatibility" Ravedude.toml and use that with the new code.

I assume in this case command-line options would override the TOML file. Would we want a warning to be printed in that case?

Hm, so let's look at all the CLI args we have right now and I'll share my thoughts for each one:

• -c, --open-console: Field in Ravedude.toml, we might want true/false overwrite from CLI to be possible.
• --debug-avrdude: Keep as is, no field in Ravedude.toml
• -b, --baudrate <baudrate>: Field in Ravedude.toml, can be overwritten from CLI
• -P, --port <port>: Optional field in Ravedude.toml, can be overwritten from CLI or env ($RAVEDUDE_PORT)
• -d, --reset-delay <reset-delay>: Field in Ravedude.toml, can be overwritten from CLI
• <BOARD>: The chosen board should only be passed on CLI in legacy mode. When Ravedude.toml is present, passing a board should result in an error.

Generally, I wouldn't display warnings for the overrides. I'd only show warnings for deprecated things or errors when someone combines legacy invocations with a new Ravedude.toml.

Does this signify approval? Because I'd rather not change things just for the sake of changing things. If mandatory TOML configs don't benefit the project in any way, they shouldn't be implemented.

Yeah, let's do it. For the purpose that ravedude wants to fulfil, this seems the most appropriate path.

although I'm leaning towards kebab case to mirror Cargo.toml

Same here :+1:

---

Thanks again for working on this! I'm excited to see the result :) This will make ravedude so much more usable and finally elevate it beyond the ugly hack that it currently is...

[Creative0708]

Whew! Yeah, no worries, I've been busy as well...

Anyways, a ton of stuff is complete! Additionally, it's now in a state where this could be merged, although a few (minor) things differ from your suggestions. Listing them off:

• Board inheritance now works! Something like

  [general]
  open-console = true
  serial-baudrate = -1
  port = "/dev/ttyUSB0"
  
  [board]
  name = "Arduino uno without automatic reset, for some reason"
  inherit = "uno"
  
  [board.reset]
  automatic = false
  message = "Take a minute to admire the coolness of the RAVEDUDE!"

  is valid. Everything (well, everything that can be overriden) in the RavedudeConfig struct is now an Option<T>, and ravedude "merges" the user-provided configuration to one of the hardcoded boards in this case.

• As in the TOML above, entries for the command-line options are now present.

• Just using a named board works too:

  [general]
  open-console = true
  port = "/dev/ttyUSB0"
  
  board = "uno"

• All the hardcoded boards have been translated it to src/boards.toml. Currently the entire file is just include_str!'d into the binary because I can't seem to find a way to embed the actual configs into the program instead of parsing the toml at runtime (static-toml looks promising, but it only returns a variable TOML mapping, not a specific, validated type). There's a test added to make sure the boards are valid, though. Maybe add that to CI if we can't find a better solution?

• A warning is displayed for people still using the old format:

  $ # cargo runner is `ravedude uno -P /dev/ttyUSB0`
  $ cargo run
  Warning: Passing the board as command-line argument is deprecated, use Ravedude.toml instead:
  
  # Ravedude.toml
  [general]
  port = "/dev/ttyUSB0"
  
  [board]
  inherit = "uno"
  
  # ravedude continues like normal...

  Warning text/format is up for discussion, obviously.

• Various other errors are displayed if the user doesn't include a port, avrdude programmer, avrdude part number, passes a board name as a command-line argument while using Ravedude.toml, doesn't provide a board config, etc.

Additionally, using Ravedude.toml I have successfully uploaded a rust program to the ATtiny85 through the Arduino Nano!

Anyways, yeah, unless we want to make changes to any of this (or if there are bugs), this PR is ready to merge! 🎉 (Not saying that it should be merged right now, but that it can be... there are also merge conflicts that need clearing up)

Thanks again for working on this! I'm excited to see the result :)

No problem! ravedude (and the entirety of avr-hal, really) makes my life a lot easier when using Rust for AVR-related projects! Happy to help :D

[Creative0708]

Oh yeah ­-- We should also probably modify stuff like the avr-hal template to use Ravedude.toml, along with adding options for completely custom setups.

[Creative0708]

Hi, again, sorry for the delay. I've made modifications to the code and left comments based on your feedback. Thanks!