Severson-Group / AMDC-Hardware

Circuit board designs for controlling advanced motor systems
http://docs.amdc.dev/hardware
33 stars 3 forks source link

AMDC REV F Hardware #228

Closed DivyaMendpara closed 10 months ago

DivyaMendpara commented 12 months ago

This PR details modification made in AMDC Rev F board

Following changes were made for addressing issue #189

image

Following changes were made for addressing issue #227

image

Following changes were made for addressing issue #226

image

Following changes were made for addressing issue #211

npetersen2 commented 11 months ago

@codecubepi can you also review this PR?

npetersen2 commented 11 months ago

@DivyaMendpara please update the Project Parameters to change the Revision to be F.

You'll notice on the schematic sheets lower-right box it still says E. Once you update the Project Parameters, it should update on all sheets.

codecubepi commented 11 months ago

@codecubepi can you also review this PR?

  • Open in Altium
  • Verify changes to modified files
  • Review each issue in the REV F milestone and add an issue comment per your review (see the REV E milestone and issue reviews as examples)
  • Run checks to ensure design passes DRC for PCB and schematic review

@npetersen2

I will check it out!

codecubepi commented 11 months ago

@DivyaMendpara

This looks pretty good to me! Thank you for addressing #211 for me!

I do have some questions on the routing for #226 :

Is there a reason this trace for GPIO4_IN3_P wraps around the via? Is this a length-match? If it's not, I think we could take the trace directly to the via (like the dashed red line). If it is a length-match, disregard. image

Why is there a trace loop on GPIO1_IN2_N? image

@npetersen2

I went through all the issues under the REV F milestone and added my comments.

Questions:

image

npetersen2 commented 11 months ago

@codecubepi thanks for the review! Regarding your comments:

  1. @DivyaMendpara will address the routing issues for GPIO and the loop traces
  2. Since GPIO is rather slow (10-20 or less MHz) and the physical length difference of the traces is reasonable small, I don't think length matching is required here. So, as long as the trace routing is "reasonable" i.e., no loops, no odd things, it should be fine.
  3. Yes, the DRC has some rule violations for the solder mask and silkscreen. These are tolerable... ideally we would invest the time to update all "bad" footprints to reduce the DRC rule violations to zero, but I don't think this is worth the effort. In REV E, these violations existed and the assembly went without issue. The footprint updates (your issue and my changes to the PicoZed footprint) in REV F reduced the number of violations by 2x, so on the right path :)

@DivyaMendpara once you have addressed the trace routing issues @codecubepi identified, please ask @codecubepi to review once again the routing. Then, upon approval, you can generate the REVxxxxF/ design output folder and add it to git in this PR. We will then review the compiled output to make sure it is correct and then approve for ordering. Please follow the directions here for how to export the design.

DivyaMendpara commented 11 months ago

@npetersen2 and @codecubepi I created AMDC Rev F folder and uploaded design outputs (schematics, BOM, etc.) from Altium Designer. Please review and give your feedback.

DivyaMendpara commented 11 months ago

@DivyaMendpara which capacitor part number did you change in the previous commit?

@npetersen2, I changed below capacitor value (now it is 0.1uF). Total qty (40+45). <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40">

45 | 1276-1012-6-ND | C6, C7, C9, C10, C11, C12, C14, C15, C16, C17, C19, C20, C21, C23, C31, C32, C33, C34, C65, C66, C67, C68, C69, C70, C92, C93A, C93B, C93C, C93D, C93E, C93F, C93G, C93H, C94A, C94B, C94C, C94D, C94E, C94F, C94G, C94H, C118A, C118B, C119A, C119B | Capacitor | 0.1uF | 0603-CAP -- | -- | -- | -- | -- | -- 40 | 1276-1012-6-ND | C24, C25, C27, C28, C29, C30, C71A, C71B, C71C, C71D, C71E, C71F, C73A, C73B, C73C, C73D, C73E, C73F, C74A, C74B, C74C, C74D, C74E, C74F, C76A, C76B, C76C, C76D, C76E, C76F, C99, C100, C101, C102, C107, C110, C111, C112, C113, C114 | Capacitor | 100nF | 0603-CAP

codecubepi commented 5 months ago

@npetersen2 @DivyaMendpara

It looks like the following issues in this repository were never closed after this was merged:

Can we go ahead and close these now?

npetersen2 commented 5 months ago

Thanks @codecubepi , I went ahead and closed the issues you linked above.