Bikey-Wagon Learnings

[mawildoer]

Commit per change

Checklist

Please read and execute the following:

• My code follows the coding guidelines of this project
• My PR title is following the contribution guidelines
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (if applicable)
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I ran Black to format my code

Code of Conduct

By submitting this issue, you agree to follow our Code of Conduct:

• I agree to follow this project's Code of Conduct

[mawildoer]

Ready to merge

[iopapamanoglou]

Looks for the most part good.
I guess the NetTie was made before fab ll?

[iopapamanoglou]

init in fab ll is purely for argument passing
split into init and preinit

    def __init__(
        self,
        width: Size,
        interface_type: type[T] = Electrical,
    ) -> None:
        super().__init__()
        self._interface_type = interface_type
        self._width = width

   def __preinit__(self): ...

[mawildoer]

Wait though, if __preinit__ is called before __init__ then how can I access _width during my static configuration?

[iopapamanoglou]

The order is init, annotations, d_fields, rt_fields, preinit, postinit
I can see where the confusion comes from.
The pre/post refers to pre/post base class construction

[iopapamanoglou]

static traits should be in the class body

attach_to_footprint: F.can_attach_to_footprint_symmetrically
designator_prefix = L.f_field(F.has_designator_prefix_defined)("H")

@L.rt_field
def footprint(self):
    width_mm = NetTie.Size(self._width).value
    return F.has_footprint_defined(
                KicadFootprint(
                    f"NetTie:NetTie-2_SMD_Pad{width_mm:.1f}mm", pin_names=["1", "2"]
                )
            )

[iopapamanoglou]

No need to do the self.unnamed =, it's done by self.add automatically

[iopapamanoglou]

Use times:

self.add(times(2, self._interface_type), "unnamed")

[iopapamanoglou]

Avoid direct imports from faebryk.library, use F.

[mawildoer]

I'm pulling this into my project for now and separating the PRs.
Here's another draft for it specifically: #39

[iopapamanoglou]

Shouldn't the width be a parameter instead of constructor argument?

[mawildoer]

Ideally, but then I'd need to defer construction until later to make the footprint trait work.
Until that's all working this seems simpler and clearer

[iopapamanoglou]

You don't need delayed construction, because you are not constructing anything. Adding a trait is always possible.
You can use is_implemented in the trait to check whether the width is 'ready' for making a footprint.

[iopapamanoglou]

width: TBD[Quantity]

...

@L.rt_field
def footprint(self):
   class _(F.has_footprint):
      @staticmethod
      def get_footprint():
          value = self.width.get_most_narrow()
          assert isinstance(value, Constant)
          width_mm = value ...
          return F.KicadFootprint(f"NetTie:NetTie-2_SMD_Pad{width_mm:.1f}mm", pin_names=["1", "2"])

      @staticmethod
      def is_implemented(_self):
          return isinstance(self.width.get_most_narrow(), Constant)

   return _()

[iopapamanoglou]

data: Iterable[T]

[iopapamanoglou]

see comment in FuncSet about multidict / hash uniqueness

[iopapamanoglou]

I think iter returns Iterator[T], not sure tho

[iopapamanoglou]

Nice thank you added tests!
I'd add one for non default hash func that has collisions: e.g `lambda x: id(x) % 10'

[iopapamanoglou]

See comment in visualize.
Having a poetry dep group for this and then check if its there and if not print: install via poetry ... might be cleaner

[mawildoer]

Yeah, this one's a little weird. Not sure I love the offer_missing pattern for the deps at all.

Separately, we should shoot for a vanilla pip/pipx install working by default, not poetry. Let's detangle that where when we come by it

[iopapamanoglou]

Afaik currently everything works with a pip only setup

[iopapamanoglou]

Most things here merged in different PRs