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

♻️ Separate LCD pins from board pins #25650

Open
wants to merge 12 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

thinkyhead
Copy link
Member

Gradual migration of LCDs to separate pins file(s) using defined connectors / interfaces.

@GMagician
Copy link
Contributor

GMagician commented Apr 8, 2023

I'm not using anymore AGCM4 + RAMPS_144 but it seems that on AUX1 you have:

  VCC GND D1 D0
  VCC GND A3 A4

but VCC may be 5V/3V depending on J4 jumper (this goes also to IO pin)

AUX3 is (please note that SPI pins on AGCM4 are not like DUE but standard D50-52 may be redirected via solering pads):

 RESET GND SCK MISO VCC
  NC    5V D53 MOSI D49

AUX4 is:
VCC and not 5V

@GMagician
Copy link
Contributor

GMagician commented Apr 8, 2023

RAMPS_1.4.4_C6.pdf
LCD144-A_Rev.A.pdf

As reference I add schematic

N.B. Ramps144 has been discontinued, because of developper health issues, then AGCM4 has de facto no shields available

@thinkyhead thinkyhead force-pushed the bf2_lcd_migration_PR branch 11 times, most recently from 423e612 to e3dd99e Compare April 9, 2023 00:01
@thinkyhead
Copy link
Member Author

in #26719 you swapped the BNT buttons in the ReARM board for the CR10_STOCK_DISPLAY
can ask why?

To eliminate this pointless requirement only mentioned in the pins file:

// Requires REVERSE_ENCODER_DIRECTION in Configuration.h

@sl1pkn07
Copy link
Contributor

oh. i forgot that

thanks

@thisiskeithb
Copy link
Member

The following is what I think could be a safe way to approach this with minimal disruption for anyone:

  • Stage and commit all of the non-functional changes. This touches a lot of comments which add noise and make it harder to spot which changes in the PR deserve attention. The smaller the scope of each PR, the easier it is for someone to volunteer to review and test those changes, and actually understand what they're looking at.

  • Write a script which will set a board (one per pins file) and use the preprocessor to dump all the symbols it generates. This should be repeated for each screen which is being extracted into a new file. We have done something similar to this in the past when we were playing with adding embedded configurations, so I don't think this should be too hard.

  • Add the new LCD pins files, but ONLY include them if the board pins file has "opted-in" by declaring something like USE_COMMON_LCD_PINS. This flag could be removed once the transition is complete.

  • As pins files opt-in, re-run the script from step 2 and compare the output before/after output to check for unexpected differences. Fixing them at this stage will be a lot easier than figuring out what's wrong after people are using the change.

I'm sure there will still be some mistakes made...especially in the "weird' boards with non-standard connectors, but this should enable the majority to be transitioned without requiring any specific hardware testing.

I agree with all of this. We should also hold off on such a huge refactor until after 2.1.3.

@thinkyhead thinkyhead modified the milestone: Version 2.1.3 Jun 2, 2024
@thinkyhead thinkyhead self-assigned this Jun 2, 2024
@thinkyhead thinkyhead added this to the After 2.1.3 milestone Jun 2, 2024
@thinkyhead thinkyhead force-pushed the bf2_lcd_migration_PR branch 2 times, most recently from f678106 to e50d4ca Compare June 2, 2024 19:21
@thinkyhead thinkyhead force-pushed the bf2_lcd_migration_PR branch from e50d4ca to 1157c17 Compare July 24, 2024 00:33
@thinkyhead thinkyhead force-pushed the bugfix-2.1.x branch 3 times, most recently from 37d77d6 to aa44542 Compare September 28, 2024 01:10
@thinkyhead thinkyhead force-pushed the bf2_lcd_migration_PR branch 2 times, most recently from 44caaf5 to 34c2b6a Compare December 4, 2024 00:48
@thinkyhead thinkyhead force-pushed the bf2_lcd_migration_PR branch from 34c2b6a to 2c1f016 Compare December 4, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

7 participants