The Refactor

[ryankurte]

We have a few outstanding issues that to move forward with could use a refactor of the existing codebase.

Issues:

• svd#18 - Better error handling
  • Emilgardis:feature/error-handling - @Emilgardis WIP error handling branch
• svd2rust#96 - Common traits for identical IP blocks
  • ryankurte:feature/encode - My previous attempt at implementing encode/decode w/tests
• svd2rust#186 - Proposal: User supplied scripts to add abstractions missing-from/impossible-in the input SVD.
  • should be covered by work on svd2rust#96 when we get this done

Proposal

Things to do and PRs to accept. I'll update this list from any discussion in this issue to reflect our plan and progress.

Project Level

1. Introduce Error types for svd decoding (@Emilgardis?)
2. Introduce Encode and Decode traits for svd objects (@ryankurte?)
3. Refactor all svd objects (anyone and everyone)

Once we've defined Error types and Encode/Decode traits the refactoring part should be reasonably simple, and we can accept a PR for each to avoid nightmare refactoring.

For each SVD object (ie. object implementing an SVD element)

1. Move from src/lib.rs to srv/svd/object.rs and add pub mod object; to src/svd/mod.rs
2. Update decode method to match Decode trait (and add error handling)
3. Create encode method to match Encode trait (svd#37 has many pre-written)
4. Add tests for encoding and decoding each variant for an object.

I propose that we put a freeze on changes to master while undertaking this and make these changes against a refactor branch until we reach stability again, then merge back to master in what is bound to be a spectacularly breaking change.

[Emilgardis]

Looks good, I have some thoughts however.
First, I don't think a new branch is needed for refactoring.

I also believe making the structure of the project src/svd/object.rs is a better idea than having loose files in src/

Also, just to be clear, when you say SVD object, you mean the structs in lib.rs relating to elements in the SVD spec right?

---

I'll make a pr on my branch to help advance this, I do agree that this is what needs to happen.

[ryankurte]

On thinking more it's probably simpler to interleave PRs without having a new branch, so I agree.
Also totally happy with that layout, and yep though I'm not sure how to put that more succinctly right now? Can easily add an example for clarity though.

[Emilgardis]

Work on this is now done on https://github.com/japaric/svd/tree/refactor

[musitdev]

I can help to do the refactor when error and Encode/Decode task is done.

[ryankurte]

Finished splitting the modules out into separate files and throwing the interfaces together.
Also moved the get_ methods to ElementExt as discussed on IRC (better @Emilgardis?).

There's also the issue of some types returning arrays of objects, for which we could either introduce an EncodeArray trait or add a merge(&self, e: &Element) function to ElementExt that adds the children of the provided element to self?

Still need Encode implementations along with tests for a number of the SVD objects and to clean out the last of the assert! macros, and would be happy to accept any PRs towards this @musitdev!.

[Emilgardis]

ping @ryankurte are you still working on this?

I can probably pick this up again this weekend, I'm not 100% however on what exactly is left to do.

[ryankurte]

I haven't had a chance recently, but still hoping to. Definitely not this weekend though.

Outstanding as far as I can remember:

• a decision about how to encode objects that are just a set of attributes and/or vectors of objects
  • we could have additional traits to unify it, or just implement methods for now and update them later
• a bunch of Encode implementations are missing implementations
• a bunch of objects are missing encode/decode tests.

I just had a skim through and added TODO where there are bits missing, so find all should highlight everything that's left.

[ryankurte]

I've done most of the refactoring bits, but a lot of encode work to do still.

How would y'all feel about putting encode behind an unproven feature gate so we can get back to master / a state we can release from, and work on encoding and testing bit by bit?

[Emilgardis]

I would be okay with that approach. Having the error handling and getting rust-embedded/svd2rust#193 implemented is something we've been waiting on for a while.

[Emilgardis]

#53 is merged 🎉 , closing issue.