support for other chips from st25r39xxx family

[kkrolczyk]

This structure has been chosen for this WIP commit only - to show clearly how many places would need to be cluttered with feature flags.
I guess approach no 2 makes more sense, hopefully it wins ;)

EDIT: second commit is approach 2 from below. As expected - would require slightly more changes, and some methods to be pub, but also bring 2 benefits: stubs, to see structure at first glance, and relative ease to add new chips (st25r3916b ;))

1. approach no 1:

• kind of like ./ci.sh indicates, that "st25r3916" is first class impl, and all others impls need to remove defaults and provide their own impl, when incompatible/different. It the reference st25r3916 would live in lib.rs, while others as modules.

• Cumbersome, however requires the least changes in the current codebase - slight relocations and some feature flags.
  Separate code for s25r3911b lives in nested module.

• I don't prefer this solution, but it's 1st choice in PR - to show how many changes are needed to code, for them to be compatible, coexisting. Minimal (?) changes to original, and st25r3911b modified only to get the code even to compile, but unlikely to work.

2. approach no 2:

• "st25r3916", "st25r3911b" and any future others, are treated equally. Needs facade in lib.rs to detect chip? Needs moving lib.rs code to lib_st25r3916 (or sth like it). Possibly - if desired, have mocks to allow building with empty feature set, default = [] without "Commands" and "Regs" specific to chip Might be the best approach, however would require substantial changes to current code.

3. approach no 3:

• impls live in separate crates (maybe using some sort of common st25r39)
  • reduces burden of feature-cfg-disables.

@Dirbaio - discussion was here
https://matrix.to/#/!YoLPkieCYHGzdjUhOK:matrix.org/$jLJeWLyptxa4kbQcl-j2hKoha9DeI_Ta6m_X0t2wmhw

[Dirbaio]

thanks for the PR! Option 2 sounds good to me, it makes sense to treat all chips equal and force the user to choose one. Some feedback:

• We can't copypaste lib_st25r3911b, lib_st25r3916. it's quite a bit of code, it's not going to be maintainable long term as we add more chips.
  • a lot of the differences are due to different naming of the registers. I'd suggest editing the yaml to make the naming the same, even if it doesn't exactly match the datasheet.
  • The actual differences that aren't due to register naming can be handled with cfg's.
• I would avoid the "stubs", it's extra complexity with little benefit. IMO it's OK for compilation to fail if you don't select a chip.
• the commands enum could probably be deduplicated as well.

[kkrolczyk]

@Dirbaio thanks for the review! Sure, i'll apply the changes, cleanup the commits a bit and ping you back when it's done. It might be a couple days though, as i'll have a quick holidays.

PS.

• right, the copy paste between chips was mainly to just to see how much code / logic could be shared anyways. It will probably take some time to get it right, but should be easier when you're fine with the structure. Definitely didn't intend it to stay like this.
• Stubs will be removed then (i thought this might give someone an insight, trait-like compact view)
• Commands enum - this differs between chips, so i'm not sure how would you deduplicate, or did you mean - also with cfg's?

[Dirbaio]

is differs between chips, so i'm not sure how would you deduplicate, or did you mean - also with cfg's?

yes

[Dirbaio]

There's still a lot of code duplication in lib_st25r3911b.rs and lib_st25r3916.rs.

Also could you not include the picc_cmds change? It's unrelated to the cope of this PR which is "add st25r3911b support", we can do it later. This PR is big enough already, reviewing lots of unrelated changes is impossible.

[kkrolczyk]

Summary of the changes - so far.

• rnfc-st25r39 crate features added: st25r3911b, st25r3916. Adjusted ci.sh/examples.

• regs.yaml - st25r3916 were moved to regs dir, new one for st25r3911b added
  as requested in comments - if simple register rename suffice,
  then register was renamed in generated rs code to match st25r3916
  generated regs.rs show lots of changes, due to alphabetical sorting

• rnfc-st25r39/src/lib.rs was moved to rnfc-st25r39/src/impls/lib.rs
impls/mod.rs commands/mod.rs and impls/interrupts.rs extract some of its parts

• some different parts between chips could have been separated more;
  however - as requested - where possible kept in one file, with feature gates

• no logic changes (especially to st25r3916) were introduced

Somehow GH didn't want to treat src/impls/lib.rs as renamed (and modified) src/lib.rs
thus not really showing proper diff - shows them as created/removed... 🤷‍♂️
Maybe i'll need to squash the commits first.

@Dirbaio not sure what's your preference, so i could make this least possible burden to review?

• rebase & force push, maybe with separate git mv from to, to make it realize it's mostly the same file
• squash
  Or do you prefer to simply compare yourself git diff master -- rnfc-st25r39/src/lib.rs rnfc-st25r39/src/impls/lib.rs or any other preferred way.