search

MarlinFirmware / U8glib-HAL

Customized U8glib for use in Marlin 2.0
Other
45 stars 33 forks source link

Massive styling, indentation and samd support #15

Closed GMagician closed 5 years ago

GMagician commented 5 years ago

Massive code indentation Massive code styling Remove compile warnings Add samd51 support

GMagician commented 5 years ago

Note: one weird thing I noticed in file u8g_dev_ssd1306_128x64.cpp is that function u8g_dev_ssd1306_128x64_adafruit2_init_seq is used in u8g_dev_ssd1306_128x64_fn even if it's not the desired init sequence. In u8g_dev_ssd1306_128x32.cpp this doesn't happens!

I kept behaviour but maybe someone should take a look at it

thinkyhead commented 5 years ago

We don't use the master branch for Marlin currently, but rather the dev branch. I will try to rebase this on dev but if it doesn't work you may have to re-roll this.

thinkyhead commented 5 years ago

You should not re-style the u8g code. There is no point in doing so. We want it to mostly match the original source so that our functional changes and additions stand out.

GMagician commented 5 years ago

@thinkyhead that's ok, I'll only add samd51 parts. What about note I wrote?

edit: and warnings fix?

thinkyhead commented 5 years ago

I kept behaviour but maybe someone should take a look at it

I'm going to sleep for a while. But if you have some time, take a look through the latest u8glib code at the original github repo and see what changes have been made in that area. Maybe the confusion was cleared up at some point.

Your effort to clean up the code is much appreciated, and I have wanted to do such a thing myself. But it tends to make changes versus the original code hard to follow, and also I didn't want to use up a whole day when there's so much else to do. Please hold on to those formatting changes, and after a little while we probably ought to merge them, if only to make working with the u8g code less painful.

GMagician commented 5 years ago

ok, what is need to me is samd51 integrarion (if allowed) and I'll post just that. My only question is about initilization function that is called despite is not "wanted"

GMagician commented 5 years ago

also it seems Marlin uses bugfix, not dev. Am I wrong?

GMagician commented 5 years ago

Your effort to clean up the code is much appreciated, and I have wanted to do such a thing myself.

If you wish I may do a "indentation" only PR

thinkyhead commented 5 years ago

also it seems Marlin uses bugfix, not dev. Am I wrong?

That is correct. The release of 2.0.x will point to master, the Marlin repo will get a new dev-2.0.x branch pointing to the dev branch here, and the bugfix-2.0.x branch will keep pointing to bugfix.

thinkyhead commented 5 years ago

If you wish I may do a "indentation" only PR

After I evaluate the SAMD51 patch, assuming all is well, I'm okay with merging all of the changes here. If I can arrange the changes in a way that makes it easier to pick out our custom changes, then I will do that after the patch.

GMagician commented 5 years ago

@thinkyhead I'll leave code available if/when you want to check/use it, but please take a look at what I wrote on "Note" in 2nd post. It seems that current code uses a different init sequence just for 1 function while the whole code uses the "activated" one. In detail, current code uses adafruit3_init while u8g_dev_ssd1306_128x64_fn function uses adafruit2_init. Maybe this is ok, maybe this is a "forgotten" code change that need to be addressed

thinkyhead commented 5 years ago

In detail, current code uses adafruit3_init while u8g_dev_ssd1306_128x64_fn function uses adafruit2_init. Maybe this is ok, maybe this is a "forgotten" code change that need to be addressed

Are you sure this is a U8glib-HAL change and not an issue from the original U8glib?