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

[RFC] Organization of Extensible UI #13201

Closed
marcio-ao opened this issue Feb 18, 2019 · 15 comments
Closed

[RFC] Organization of Extensible UI #13201

marcio-ao opened this issue Feb 18, 2019 · 15 comments
Labels
C: Build / Toolchain T: Development Makefiles, PlatformIO, Python scripts, etc.

Comments

@marcio-ao
Copy link
Contributor

@thinkyhead: Currently, the FTDI Touch Screen UI code is housed on the LulzBot code repo, where it is integrated into our FW. In order to make it more accessible for folks who may want to use it as part of Marlin for other printers, I think it makes sense for there to be a separate repo under the "MarlinFirmware" header.

Would it be possible for there to be such a repo added? And if so, could I be made a maintainer for it? If such a repo was available, I would keep the repo in sync with modifications made to our own FW and I could also add more documentation to it, and possibly address some of the concerns raised in #12096 (comment)

Thoughts?

@thinkyhead
Copy link
Member

thinkyhead commented Feb 25, 2019

Please use your existing fork of Marlin and make a new branch. Be sure to update your fork's copy of bugfix-2.0.x first, and create your new branch as a copy of bugfix-2.0.x. Once you have a basic integration completed, post a new PR. That PR will form the basis for further work on the feature.

@InsanityAutomation
Copy link
Contributor

I guess the question stems more with how to handle examples for EXTui. Should the lib folder be placed in the machine example configuration with configuration and configuration_adv or should the EXTui chunks be managed in a separate repository to avoid growing size.

Which also brings up a question, should the config dir with default and examples be moved up above or into the Marlin folder for better visibility to end users?

@thinkyhead
Copy link
Member

thinkyhead commented Mar 3, 2019

ExtUI-based devices should be adaptable to any machine with enough free pins, and should not be associated with configurations. I do believe they should live within the lcd folder as they are a part of the compiled sources.

should the config dir with default and examples

Probably, I have been considering it, since they are not part of the actually-compiled sources.

@thinkyhead thinkyhead added T: Development Makefiles, PlatformIO, Python scripts, etc. C: Build / Toolchain labels Mar 3, 2019
@boelle boelle mentioned this issue Jun 22, 2019
20 tasks
@InsanityAutomation
Copy link
Contributor

FWIW we have several extui screens in the main code base now... Seems to have migrated away from this discussion a bit. I still have a long term concern for maintainability.

@marcio-ao
Copy link
Contributor Author

@InsanityAutomation: Can you point me to the code for such screens? Just curious...

@InsanityAutomation
Copy link
Contributor

InsanityAutomation commented Jul 24, 2019

In head we heave

//=============================================================================
//========================== Extensible UI Displays ===========================
//=============================================================================

//
// DGUS Touch Display with DWIN OS
//
//#define DGUS_LCD

//
// Touch-screen LCD for Malyan M200 printers
//
//#define MALYAN_LCD

And working mostly but needing another maybe 3-4 days dedicated work before ready to submit

https://github.com/InsanityAutomation/Marlin/tree/CrealityDwin_2.0

Note itll take awhile to get those dedicated days...

@marcio-ao
Copy link
Contributor Author

It seems like most of these are relative small, about 2k lines of code max. Our UI is a behemoth in comparison, weighing in at 16k LOCs.

@marcio-ao
Copy link
Contributor Author

marcio-ao commented Jul 24, 2019

It looks like most of these UIs off-load most of the actual work to a separate processor, with its own FW that draws the graphics and handles interaction. Ours is maybe the only one where the MCU itself is drawing the contents of the UI and handling the event loop, which is what makes it so large.

Still, if these other screens are making it into the source tree, I feel like ours should as well...

@InsanityAutomation
Copy link
Contributor

Right, mine is bigger than these as well but the same DGUS hardware as the first above. I agree with youre statement there of they should be managed the same way yours is. Either in the machine example configs, seperate repo of Extui Libraries, or least optimal IMO in the main source. Consistency regardless.

@marcio-ao
Copy link
Contributor Author

It also looks like the Malyan LCD is using some methods of the thermalManager, rather than using the abstraction provided by ExtUI. This is something I worried would happen, as there is nothing preventing folks from calling the underlying code.

@marcio-ao
Copy link
Contributor Author

@InsanityAutomation: So it looks like there isn't very much consistency about where the source code for the various displays are stored. Right now, here's where things are located:

  • Marlin/src/lcd/extui_dgus_lcd.cpp
  • Marlin/src/lcd/extui_example.cpp
  • Marlin/src/lcd/extui_malyan_lcd.cpp
  • Marlin/src/lcd/extensible_ui/lib/Creality_DWIN.cpp
  • Marlin/src/lcd/extensible_ui/lib/Creality_DWIN.h
  • Marlin/src/lcd/extensible_ui/lib <-- This is where Lulzbot puts all our code

Maybe it would make more sense to have subdirectories under extensible_ui, one for each display:

  • Marlin/src/lcd/extensible_ui//dgus/extui_dgus_lcd.cpp
  • Marlin/src/lcd/extensible_ui/example/example.cpp
  • Marlin/src/lcd/extensible_ui/malyan/extui_malyan_lcd.cpp
  • Marlin/src/lcd/extensible_ui/creality_dwin/Creality_DWIN.cpp
  • Marlin/src/lcd/extensible_ui/creality_dwin/Creality_DWIN.h
  • Marlin/src/lcd/extensible_ui/lulzbot <-- this is where we could put our code

@marcio-ao marcio-ao reopened this Jul 24, 2019
@marcio-ao marcio-ao changed the title [RFC] Would it make sense to have a sub-project for the FTDI Touch UI? [RFC] Organization of Extensible UI Jul 24, 2019
@InsanityAutomation
Copy link
Contributor

I would agree with that placement as the second best choice aside from the example configurations.

@InsanityAutomation
Copy link
Contributor

Originally it was all internal and @thinkyhead converted it over. Seems some slipped through the cracks. Ill add it to my list to review the converted ones at some point.

@InsanityAutomation
Copy link
Contributor

@boelle @shitcreek The decision was made and theyre merged in the main branch, this can be closed.

@boelle boelle closed this as completed Oct 15, 2019
@github-actions
Copy link

github-actions bot commented Jul 4, 2020

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: Build / Toolchain T: Development Makefiles, PlatformIO, Python scripts, etc.
Projects
None yet
Development

No branches or pull requests

4 participants