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

Add: Circular display connector (bottom05flip) #380

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

Conversation

Suke0811
Copy link
Collaborator

@Suke0811 Suke0811 commented Sep 20, 2024

@Suke0811 Suke0811 requested a review from ducky64 September 20, 2024 08:13
Copy link
Collaborator

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

Lots of suggestions on electrical modeling details. I probably need to go through some of the other devices to make sure they adhere to these design principles too...

I think the footprint still needs to be made? Should be as simple as two circles (I typically have one for active area and one for mechanical outline) and a connector placement for reference. I like to indicate Pin 1 on the connector in the drawing layer so I can check pin orientation during layout.

This is the first part where the datasheet pinning is wonky, so I've been thinking of design principles for LCDs. Here's the proposal:

  • KiCad footprint pin numbers are authoritative and should be the numbers used in the HDL. If the datasheet differs from KiCad's pinning, a comment should indicate that.
  • The connector should be orientated such that the LCD is placed on the same side as the connector and facing upwards, and should provide for a bend in the FPC (which provides more positioning flexibility of the display itself).

In this case, since the display has top-facing contacts, we would want a bottom-facing FPC connector so the FPC could bend under the display. Does that seem reasonable?

edg/parts/Lcd_Er_Tft1_28_3.py Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
Comment on lines 95 to 97
self.led_res = self.Block(Resistor(
resistance=(self.pwr.link().voltage.upper() / forward_current.upper(),
self.pwr.link().voltage.lower() / forward_current.lower())))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self.led_res = self.Block(Resistor(
resistance=(self.pwr.link().voltage.upper() / forward_current.upper(),
self.pwr.link().voltage.lower() / forward_current.lower())))
forward_voltage = 2.9*Volt
self.led_res = self.Block(Resistor(
resistance=((self.pwr.link().voltage.upper() - forward_voltage) / forward_current.upper(),
(self.pwr.link().voltage.lower() - forward_voltage) / forward_current.lower())))

As we learned it turns out that subtracting the forward voltage is pretty important

There also is a way to do this in one expression (instead of splitting the lower and upper bounds) and have tolerancing work correctly though it's a bit more complex. Can discuss further, might be interesting to try.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Applied the current suggestions.

Is there like an array operations like that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Division can be done as a range operation, resistance = self.pwr.link().voltage / forward_current. The problem is that forward current is a spec so the tolerance of resistance needs to shrink, whereas range division semantics here have tolerances expanding.

There is Range.cancel_multiply, which does the right thing, but it needs to work on concrete values instead of RangeExpr. I'm thinking of maybe adding RangeExpr.cancel_multiply, which can then be used here. Still thinking of syntax, maybe resistance = (1/forward_current).cancel_multiply(self.pwr.link().voltage)? Possibly needs a better name too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does that do element-wise multiplication? what does cancel mean here?

edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

Finally circling back to this, was swamped...

Overall mostly looks good, one more comment inline.

Question though: does the device outline footprint and connector match? Right now it's using a Fpc050Bottom, bottom-entry connector, which means the FPC is tucked underneath the display right? But the device outline footprint has the FPC in the top-entry configuration?

The other OLEDs have shown the FPC tucked underneath, so there wasn't any issues there, but here we might have to deviate since that's not part of the drawing. I'd probably prefer keeping it Fpc050Bottom and adding an arbitrary bend point on the FPC on the device outline footprint and to be stylistically consistent with how the other OLEDs have indicated tucked-under FPCs.

edg/parts/Lcd_Er_Tft1_28_3.py Outdated Show resolved Hide resolved
@Suke0811
Copy link
Collaborator Author

Question though: does the device outline footprint and connector match? Right now it's using a Fpc050Bottom, bottom-entry connector, which means the FPC is tucked underneath the display right? But the device outline footprint has the FPC in the top-entry configuration?

I think they should still match cause bending FPC won't change the pin order, so the 'dot' should be on the same side regardless?
The footprint would have a shorter FPC length if tucked.

@ducky64
Copy link
Collaborator

ducky64 commented Dec 12, 2024

I think the footprints should be self-consistent - so with Fpc050Bottom and for the screen to face up, it needs to be tucked under (which shorten the FPC indicated on the footprint). Check out edg:Lcd_Er_Oled0.96_1.1_Outline for an example of how I've done it stylistically. It shows both the bent edge of the FPC as well as the connector edge underneath the device.

Since the datasheet doesn't have a bend point specified, maybe just pick halfway down the FPC?

@Suke0811
Copy link
Collaborator Author

Suke0811 commented Dec 12, 2024

oh, I see that makes sense. Like this? The half way bend?
image

Or do you want to tucked under the screen, so the connector end is approximately around the center of the screen

@ducky64
Copy link
Collaborator

ducky64 commented Dec 17, 2024

Yeah, I think that's good.

No particular preference from me and I'm not aware of any conventions, so halfway should be good.

@Suke0811
Copy link
Collaborator Author

okay the new outline is up with a45e893
Anything else for this component?

Copy link
Collaborator

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

Hm, it seems the dimensions are off? I think what was a diameter on the datasheet got entered as a radius on the footprint?
image

Also, are you sure about the connector width?
image

Datasheet seems to indicate it's 8mm wide, and somewhat off center too?
image
(I like to make sure this is correct because sometimes I use this to check against the connector in the layout to make sure the pins and sizes line up)

Also a few inline suggested fixes to metadata.

But almost there!

@@ -0,0 +1,27 @@
(footprint "Lcd_Er_Tft1_28_3_Outline" (version 20221018) (generator pcbnew)
(layer "F.Cu")
(descr "Outline for the ER-OLED028-1 series OLEDs. Connector dimensions (on User.Comments layer, with FPC folded over underneath the board, contact facing away from module) is from the Outline Drawing.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(descr "Outline for the ER-OLED028-1 series OLEDs. Connector dimensions (on User.Comments layer, with FPC folded over underneath the board, contact facing away from module) is from the Outline Drawing.")
(descr "Outline for the ER-TFT1.28-3 series LCDs with capacitive touch. Connector dimensions (on User.Comments layer, with FPC folded over underneath the board, contact facing away from module) is from the Outline Drawing.")

(effects (font (size 1 1) (thickness 0.15)))
(tstamp f2e2f675-ba12-4850-bfa4-ec3fe8671742)
)
(fp_text value "Lcd_Er_Oled028_1_Outline" (at 0 -0.5) (layer "F.Fab")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(fp_text value "Lcd_Er_Oled028_1_Outline" (at 0 -0.5) (layer "F.Fab")
(fp_text value "Lcd_Er_Tft1_28_3_Outline" (at 0 -0.5) (layer "F.Fab")

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.

2 participants