wokwi / wokwi-elements

Web Components for Electronics and IoT Parts
https://elements.wokwi.com/
MIT License
186 stars 52 forks source link

feature: VID2805 Dual Stepper Element #137

Closed bonnyr closed 2 years ago

bonnyr commented 2 years ago

Adding a new element to represent a common dual stepper - the VID28-05

The part looks like this: image

The component looks like this (top view):

image

and with shorter hands:

image

The part has two sets of the typical stepper pins (A-/A+/B-/B+)

One note is that I am not sure whether the pinInfo coords match the position in the drawing or how to make sure they do. Another note is that on storybook, I am not sure how to create two group of controls, one for each stepper hand and be able to use these controls when initializing a class representing each of the hands

urish commented 2 years ago

Error message from Netlify:

4:11:04 AM: src/vid2805-dual-stepper-element.ts(88,27): error TS2550: Property 'replaceAll' does not exist on type 'string'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2021' or later.
4:11:04 AM: src/vid2805-dual-stepper-element.ts(89,27): error TS2550: Property 'replaceAll' does not exist on type 'string'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2021' or later.

You can probably see the same errors if you run npm run build locally.

replaceAll is newer JavaScript function, and isn't supported yet by all browsers. An alternative would be to use someString.split("search").join("replace")

bonnyr commented 2 years ago

Thanks for the clarification @urish

This now builds correctly. There's the issues of:

  1. I am not sure whether the pinInfo coords match the position in the drawing or how to make sure they do.
  2. In storybook, I am not sure how to create two group of controls, one for each stepper hand and be able to use these controls when initializing a class representing each of the hands - currently the controls do not really translate into a change in the selected template

Any help here would be appreciated

urish commented 2 years ago

I am not sure whether the pinInfo coords match the position in the drawing or how to make sure they do.

Look at the Debugging Pin Info section of the Contributing guide to learn how to test it

In storybook, I am not sure how to create two group of controls, one for each stepper hand and be able to use these controls when initializing a class representing each of the hands - currently the controls do not really translate into a change in the selected template

I'm not sure if storybook supports groups of controls - you can take a quick look at their docs / examples and see if they do. Otherwise, you can probably use a prefix to separate the controls (e.g. hand1Something and hand2Something). Once I have an opportunity to look at the your code (probably some time next week), I will have more context to help.

bonnyr commented 2 years ago

@urish pinInfo stumps me...

The SVG for the element is using specific size in mm, the pins are drawn using relative path operations and are scaled by mmToPix :

<svg id="Layer_2" xmlns="http://www.w3.org/2000/svg" width=70mm height=70mm viewBox="0 0 180 180">
    <defs>
       ...
    </defs>
    <g id="Layer_1-2" transform="translate(0,${trY})">
        <g id="scaled_body">
          <g id="pins" transform="scale(${mmToPix})>
          <path id="pin-1" class="cls-3" d="m 58 0 v 5 c 0 2 2 2 2 0 v -5 z" />
          <path id="pin-2" class="cls-3" d="m 65 0 v 5 c 0 2 2 2 2 0 v -5 z" />

When returning pin info with the x coordinate the same as that specified in the path, the debug view shows the dots too close together:

  get pinInfo(): ElementPin[] {
    const { x, y, innerOff, outerOff, trY} = this.coords();

    return [
    { name: 'A1+', y: trY, x: 58 * 1, number: 1, signals: [] },
    { name: 'A1-', y: trY, x: 65  * 1, number: 2, signals: [] },
    { name: 'B1+', y: trY, x: 72  * 1, number: 3, signals: [] },
image

When scaling the information in pinInfo by mmToPix the dots are too spaced too wide apart:

 get pinInfo(): ElementPin[] {
    const { x, y, innerOff, outerOff, trY} = this.coords();

    return [
    { name: 'A1+', y: trY, x: 58 * mmToPix, number: 1, signals: [] },
    { name: 'A1-', y: trY, x: 65  * mmToPix, number: 2, signals: [] },
image

I've pushed the code that depicts that - any help would be appreciated.

bonnyr commented 2 years ago

@urish - managed to get the component and pins aligned and correct size - are you able to look at the component and accept/reject?

urish commented 2 years ago

Thanks! "ornate" is cool. I'll hope to go over the actual changes soon!

urish commented 2 years ago

Thanks again for making the changes!

Looking at it, here are some issues which will affect the usability on Wokwi:

  1. The bounding SVG element is too big. We need it to be as tight as possible, as Wokwi uses this when drawing the selection rectangle around the element. image

  2. The element and pins moves when you change the arm length. Let's horizontally center the element within the SVG, so it stays in the same places (and the pins too), regardless of the hand length.

If we made the maximum hand length shorter (e.g. 50 instead of 70), we'd need to leave less padding around the element. I'm not super concerned about the current amount of padding, so if there's a good reason to keep it at 70, that's fine too.

bonnyr commented 2 years ago

The bounding SVG element is too big. We need it to be as tight as possible, as Wokwi uses this when drawing the selection rectangle around the element.

and

The element and pins moves when you change the arm length. Let's horizontally center the element within the SVG, so it stays in the same places (and the pins too), regardless of the hand length.

I had to make this as large as it is to account for the longer arms - they extend beyond the base. The use case for making this element this large is when placing a number of these vertically - the hands then forms circles that are closer together.

If this is not compatible with the way Wokwi works, I am happy with your suggestion of reducing the maximum size. This will also solve the movement issue since the arms will be confined to the bounding box of the element itself and we can make the element as tight as possible.

Thoughts?

urish commented 2 years ago

I had to make this as large as it is to account for the longer arms - they extend beyond the base.

Right now, there's extra margin on the right side - the photo that I took in the previous has the largest arm size.

I see two ways to go about this:

  1. Reducing the SVG size so it has just enough padding to fit tightly when the arms are in their largest size (perhaps reducing the max arm length a bit, so we need less padding)
  2. Introducing some mechanism to tell Wokwi about the true bounding box of the element (similar to what we have in pinInfo). That will probably also be useful for #119 if it ever lands.

The first option will be faster, as the second one needs some changes on the Wokwi side as well.

In any case, the pin positions / motor body should stay static even when you change the arm length.

I hope this all makes sense to you.

bonnyr commented 2 years ago

@urish - another update, addressing the comments I believe

urish commented 2 years ago

Thank you!