Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Daybreak rework #24

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

Daybreak rework #24

wants to merge 55 commits into from

Conversation

KnockbackNemo
Copy link

@KnockbackNemo KnockbackNemo commented Oct 17, 2024

Quality Assurance Checklist

To make reviews more efficient, please make sure the board meets the following standards and check everything off once the board meets the quality check. Once everything has been checked, the assigned reviewers will begin the review process. Edit this description to check off the list.

There are exceptions with all guidelines. As long as your decisions are justified, then you are good! Contact the reviewers or the leads about any exceptions.

Minimum Prerequisites

  • Pushed changes to branch.
  • The board passes ERC and DRC.
    • Note some ERC warnings are acceptable.
  • All traces are routed.
  • Units are in metric.

2D Spacing

  • The components are spaced out at an optimal distance.
    • All components can be easily hand-soldered.
    • IPC-SM-782A Standard requires a minimum distance of 1.0mm from edge to edge.
  • Components that are in parallel with each other are spaced out at an equal distance when possible.
  • The components are aligned with each other when possible.
  • Components are grouped based off of functionality.
    • E.g. all components for CAN should be grouped.
  • Bypass capacitors are less than 1cm away from their respective IC's power pins.

3D Spacing

  • Layout of components have been placed with mounting location and usage of the board in mind.
    • Are PCBs going to be stacked on above/below this board? Are tall components placed accordingly?
    • Are buttons and lights easily accessible and viewable?
    • When mounted into the car, are the heights of the components and connectors accounted for?
  • Location of connectors and wires going out of the board will prevent messy wiring.
    • Are all the wires that are going in the same direction placed on the same edge of the board?

Components

  • Custom footprints have been double checked with the datasheet.
  • Pin 1 of the footprint is labeled in some way.

Copper Layer

  • The trace widths and trace clearances are greater than JLCPCB's minimum requirements.
  • The trace lengths are as short as possible.
    • Can there be a more optimal route if you go to another layer?
  • Each trace's width is capable of handling the expected current flow.
    • Use PCB width calculator to calculate trace width.
  • *No sharp corners. No trace angles equal to or less than 90 degrees.
  • Traces are in parallel with each other when possible.
    • E.g. traces connected between an IC and MCU can be placed in parallel with each other.
  • There are no random trace appendages.
  • Vias placed in copper pads are fully encompassed in the copper pads.
  • Through-hole components do not have extraneous vias.

*Not really a problem for modern manufacturing techniques but good practice and important for high speed signal integrity.

Silkscreen Layer

  • Visible text sizes are greater than JLCPCB's minimum requirements.
  • All visible text is on the silkscreen layer.
  • All reference labels of each component are not overlapping a copper pad or another component.
  • All connector pins are labeled with a meaningful and helpful name.
  • All LEDs are labeled with a meaningful and helpful name.

Edge Cut Layer

  • The dimensions of the board and the mounting holes are nice values in metric i.e. no long decimals.
  • The physical outline of the board is on the edge cut layer.
  • Edge cuts are straight and parallel with opposite edge of the board.
  • Mounting holes are aligned.
  • Corners are curved and contoured to mounting holes.

IMPORTANT NOTICE

  • I am confident that this board will be safe to use in a safety critical environment.
  • I had fun :)

Other Comments

Write any comments about the board that would help the reviewers here.

Copy link
Contributor

@Lakshay983 Lakshay983 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Today I learned that KiCAD doesn't differentiate between net labels and hierarchical labels if they're named the same thing. Pressing ~ on the net shows that KiCAD thinks they are electrically connected:
image consider renaming the name of the nets to be different.

Some ERC stuff:

  • the "Foward" pin on the MCU isn't connected to anything (misspelling go brrrr)
  • power labels are global so you don't need to have hierarchical labels. Either way, the relevant hierarchical labels aren't on the sheet.
    image

The dashboard PCB has pin 2 of the dashboard connector as BPS hazard and y'all have it as regen.

I thought we were getting rid of the 555 timer in favor of doing it in software? Doing it in hardware is also fine, was just wondering.

Make sure you update all the footprints and ask for a review on that before you start layout.

You shouldn't need to use the "SI8261ABC-IS" gate drivers, you should be able to just use the EL3H7 (right @ppatra126 ?) The gate of the mosfet doesn't sink that much current.

Right_Ind and Left_Ind are at 12V logic and referenced to GNDPWR, so you can't plug it into the gate of your mosfet since the 5V of the timer is referenced to GND (thus breaking the isolation barrier).
image

…e MinionBrdInterface so that they aren't connected. Changed Regen input to BPS Hazard light since we aren't using regen but will be alerting the driver of BPS faults.
@ppatra126
Copy link

Would it be possible to break out another 4-pin connector for MotorCAN? In case we want to try adding any lighting/pedal boards onto this gen for testing that would allow us to daisy chain CAN through controls without the leader board being a terminating end.

image
We found that this configuration probably doesn't actually work since current can be conducted in the reverse direction through the body diode. The most recent contactor driver should have a proper configuration so I would copy their schematic.

…r to motor can and disconnected shield pins for now since I haven't seen them used (will chack against old version to see what the circuit looked like), copied power-in circuit from contactor board, added optoisolator to timer and blinker light connection.
…pefully this did not involve doing anything that wasn't supposed to be done.
@KnockbackNemo
Copy link
Author

Thank you both for the reviews!

Lakshay:
I renamed the nets to be different, chose to use global power labels, and renamed/added power flags until the ERC was happy. Regen is now Bps_Hazard, the timer uses an optoisolator instead of a mosfet (since it feeds directly into another optoisolator, I don't think current will be a problem?), and the SI8261ABC-IS gate drivers are now also optoisolator drivers.

We decided we like abstracting the blinky stuff into hardware, so we plan to keep the timer unless @diyarajon prefers software.

Prat:
I added a second CAN connector and changed the type to be the same one as that used by CarCAN. I also copied the power-in input circuit from the contactor board, though I'm not sure I understand the issue/solution. If you have time, would you mind explaining how the current goes backward through the diode?

Additional changes/concerns:

  • I might be crazy, but it seemed like the optoisolators were reversed, so I flipped them all (and changed the resistor values to account for 12V logic). Could you check that they're being used correctly?
    image
  • Removed current-limiting resistor between the 555 timer and lights driver since the line feeds directly from one optoisolator to another, so the current should be within the allowable range already
  • Not sure if I copied over and adapted the contactor board power distribution input circuit correctly
    image
  • MotorCAN no longer uses the shield lines. We didn't seem to be using them to begin with, but if you could check the MotorCAN circuit, that would be greatly appreciated.
    image

Thank you very much!

@IshDeshpa
Copy link

IshDeshpa commented Nov 2, 2024

image
I would just remove the second set of potentiometers for brake/accel and only have one pot circuit per. Also if you're switching to a limit switch/pressure sensor or smth for brakes make sure that this circuit will continue to work as intended (is the pressure sensor resistive? e.g. will it work the same as a pot or will it require special circuitry)

@IshDeshpa
Copy link

IshDeshpa commented Nov 2, 2024

image
Do these just go to external LEDs? Is isolation required?
Edit: I think these are the contactors. You should probably rename/retitle the sheet.

Additionally, do we still need four contactors on this car? I would make sure the output connector here will match up with the new contactor driver board's pinout

@IshDeshpa
Copy link

image
image
Do you intend to have 8 mounting holes or is this just something left over from copy/paste

@IshDeshpa
Copy link

image
Resistor value you use here will depend on the LED you use. We typically use APTD2012LCGCK (from Standardized Mouser Components). See https://www.sparkfun.com/tutorials/219 and verify this value will work.

@IshDeshpa
Copy link

image
Where does this assumption come from? Where are the current limiting resistors going to be placed? I would make sure we have a plan for this, or if not we can just include the I-lim resistors on this board.

@IshDeshpa
Copy link

image
Would probably talk to Mika for DashboardPCB and spec out a LED (lowk it can just be one of them regular breadboard LEDs unless you really want something special)

@IshDeshpa
Copy link

IshDeshpa commented Nov 2, 2024

Make sure you update all the footprints and ask for a review on that before you start layout.

I would definitely add a P/N (part number) and footprint to every component on here. Looks like the footprints are OK but double check that they're the ones you want.

@KnockbackNemo
Copy link
Author

@FrankLiTX @ParthivS20 Our connectors currently all have the clips facing out (like the last Controls leaderboard). Should we flip them to face the inside instead?

@IshDeshpa
Copy link

image

this seems ok on first glance. you don't have to keep the question in the schematic at least, just state your justification (6ma < 60ma)

@IshDeshpa
Copy link

IshDeshpa commented Jan 8, 2025

image
Make sure these holes don't go off the edge cut border, i'd do this for all connectors (just move em in a touch)

@IshDeshpa
Copy link

image
is this a cloud or a tree

@IshDeshpa
Copy link

image
This seems a bit unwieldy. Could you put the fuse on the back and then move some traces so that this trace is a bit more direct? it just sucks this is the one thing that's like on the other end of the board

@IshDeshpa
Copy link

image
Can you explain the brake pot AND brake switch decision? why both, not sure i remember.

@IshDeshpa
Copy link

Also for the 12V traces you do have double check to make sure that they can handle the required amount of current.

Copy link

@IshDeshpa IshDeshpa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably all of my comments, it's not much but that's just because I'm not used to reviewing PCBs. If a @Lakshay983 or a @Champers5000 or a @FrankLiTX could review this for actual design errors it may be better.

@Lakshay983
Copy link
Contributor

image Can you explain the brake pot AND brake switch decision? why both, not sure i remember.

Iirc it was because Ergo didn't know 100% which option they were doing for Daybreak and it was easier to support both on the Controls side.

@Lakshay983
Copy link
Contributor

Lakshay983 commented Jan 9, 2025

Can you make J1 (the extra ADC connector) a molex connector since the pin headers aren't great for vibration

C41 is a bypass cap for the usb-uart chip and it's pretty far away, can you make it closer to the 3v3 pin of that chip

Make sure you update your board constraints: https://github.com/lhr-solar/UTSVT-KiCadLibraries?tab=readme-ov-file#standard-constraints-for-laying-out-board since I'm running DRC and it's giving me a ton of nets bridged since your clearance requirement is higher than needed

Can you fill some of the areas that aren't filled. It's mostly for ✨ aesthetic ✨
Screenshot 2025-01-08 at 10 09 27 PM
Screenshot 2025-01-08 at 10 10 01 PM
The solution usually being to group these closer traces to be in busses

I second Ishan on the 12V trace being a bit odd. I also don't like routing all the current for the lights technically next to the crystal oscillator. Maybe try routing it around the border of the board to get to motor connector. There's also the nuclear option of switching the carcan and motorcan stuff is so you don't need to route the 12V trace as much.

The brake pot trace is longer than it needs to be. I think you can move C8 slightly closer to the oscillator and you can fit the trace inside that bus of red traces to go directly into the chip
Screenshot 2025-01-08 at 10 15 16 PM

For the 3v3 traces, you can simplify a lil bit since there's some routing on the top and bottom layer going in different directions.
Screenshot 2025-01-08 at 10 18 54 PM
You could just via from that red tree branch up to the top pin.
Screenshot 2025-01-08 at 10 21 31 PM

you can get rid of this GND trace and via off of pin 47 and C11 by moving the SWDIO via out a little bit so you can form a thermal relief.
Screenshot 2025-01-08 at 10 23 17 PM

For pin 63 and 12 instead of a via and long trace you could just do a small trace to a via per pin. I could spin some wacky signal integrity reason like you want both pins to have equal traces to the GND plane but it's mostly for aesthetic and reduction of traces.

CAN routing can be improved a little bit so the signal traces aren't branching in a bunch of directions.
Screenshot 2025-01-08 at 10 36 01 PM
Flip R34, move U18 up a little bit, swap R35 and JP2 so now the can traces follow more of a path and the area can fill
Screenshot 2025-01-08 at 10 43 25 PM
With that you can also route the GNDISO trace down the middle of the chip and into the pins instead of routing it around the chip and through the resistors.
Screenshot 2025-01-08 at 10 54 52 PM
This was my idea for routing the CAN signals so there's some less long branching but also more vias. Up to y'all on what you want to do tho.
Most of this stuff applies for the motor can chip as well.

Why route the break switch through the reset button? Could be routed under the reset button mayhaps.

Motor should be GNDPWR not GND
Screenshot 2025-01-08 at 10 37 14 PM

@Lakshay983
Copy link
Contributor

Be careful about PC13-PC15. The datasheet specifies some limitations for those pins
Screenshot 2025-01-08 at 11 30 51 PM

@Lakshay983
Copy link
Contributor

Why do you have the area with no fill? You can extend the GND fill to fill that space. Also, i'd suggest making the GND plane a square that fills the entire board, and then assigning it a lower priority than your 12V and GNDPWR planes so the 12V plane will always fill where it needs to and everywhere else it'll fill GND. Just so you don't need to draw wonky shapes if you ever need to make stuff around.
Screenshot 2025-01-09 at 1 07 44 PM

@Lakshay983
Copy link
Contributor

Also i don't think you need the 2 CAN testpoints if you have those DB9 outputs, it's also fine to keep em.

@Lakshay983
Copy link
Contributor

Lakshay983 commented Jan 9, 2025

Screenshot 2025-01-09 at 1 25 12 PM

This breaklight trace is pretty small considering that it's going to power multiple lights and it's all returning through that small trace. You have a big plane to sink all that current for GNDPWR but you're limiting yourself with that trace going into the fet.

Also you can't see the labels for this cuz the connector silkscreen is blocking the L+12V label

@Cam0Cow
Copy link

Cam0Cow commented Jan 10, 2025

image
This seems overkill for what I assume is an on-board LED, or is it routed elsewhere to something beefier?

image
Are these resistor values accurate? It looks just like the old circuit, but you're wasting a decent amount of the range of the ADC which doesn't seem ideal. A weaker pull-up might make sense here.

image
Not gonna nitpick the layout since it seems Lakshay has that covered, but it seems like there's still some missing connections, like this 12V pin. Are all the other new boards two layer only as well, or just this one? It doesn't seem to necessarily need it, just curious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants