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

General cleanup, mostly MKS UI #19825

Merged

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Oct 20, 2020

Gathering changes from #19812 that are unrelated to the new Web GUI to make that PR more simplified.

  • Used #include <lv_conf.h> for external header.
  • Applied ZERO macro where possible.
  • Fixed some indentation and spacing.
  • Tweaked code for readability and conciseness.

@rhapsodyv
Copy link
Member

Removed #include "lv_conf.h" referencing non-existent header.

This header is inside the MKS LVGL repo..... It has the LVGL configs.

@thinkyhead thinkyhead force-pushed the bf2_stuff_before_19812 branch from 72b4f4f to 826fd6b Compare October 20, 2020 23:17
@thinkyhead
Copy link
Member Author

This header is inside the MKS LVGL repo..... It has the LVGL configs.

Must this header be added into Marlin before it can be located, or is it found automagically?

@rhapsodyv
Copy link
Member

This header is inside the MKS LVGL repo..... It has the LVGL configs.

Must this header be added into Marlin before it can be located, or is it found automagically?

It's in the same folder as <lvgl.h>

Explaining: LVGL needs a configuration header, like Marlin. MKS forked LVGL, changed a few things, and added that header to their repo, fully configured for their UI. So, "lv_conf.h" is provided by the lvgl library, pointed by the marlin feature.

This header contains macros with configs. Removing the marlin include can break anything? I don't know, we need to test, as include order is something sensible to macros....

@rhapsodyv
Copy link
Member

rhapsodyv commented Oct 20, 2020

Yes, I added tests for LVGL in mks_robin_nano35. But just flashing and testing to see if removing that lv_conf.h include break anything, as it's just a macro file, and without that some defaults would be chosen instead of the configs... But, it could be already been included by the lvgl files... I don't know... needs testing....

@thinkyhead thinkyhead force-pushed the bf2_stuff_before_19812 branch from ee3e2df to 5832f98 Compare October 21, 2020 00:00
@thisiskeithb
Copy link
Member

I have an MKS Robin Nano 1.2 & 2.0 that I can flash here for testing as well.

@thinkyhead thinkyhead force-pushed the bf2_stuff_before_19812 branch from d9710f7 to 6223e73 Compare October 21, 2020 00:49
@rhapsodyv
Copy link
Member

I have an MKS Robin Nano 1.2 & 2.0 that I can flash here for testing as well.

It would be good. I cannot test today. Maybe later tonight.

@thinkyhead thinkyhead force-pushed the bf2_stuff_before_19812 branch from 6223e73 to ce70e2f Compare October 21, 2020 03:37
@thinkyhead thinkyhead merged commit 072f996 into MarlinFirmware:bugfix-2.0.x Oct 21, 2020
@thinkyhead thinkyhead deleted the bf2_stuff_before_19812 branch October 21, 2020 17:45
thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Oct 21, 2020
sebsx pushed a commit to sebsx/Marlin that referenced this pull request Oct 25, 2020
Speaka pushed a commit to Speaka/Marlin that referenced this pull request Nov 2, 2020
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request Dec 9, 2020
tharts pushed a commit to tharts/Marlin that referenced this pull request Jan 6, 2021
kpishere pushed a commit to kpishere/Marlin that referenced this pull request Feb 19, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Apr 28, 2021
thinkyhead added a commit to thinkyhead/Marlin that referenced this pull request Apr 29, 2021
thinkyhead added a commit that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants