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

Massive styling, indentation and samd support #15

Closed
wants to merge 1 commit into from
Closed

Massive styling, indentation and samd support #15

wants to merge 1 commit into from

Conversation

GMagician
Copy link
Contributor

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

Massive code indentation
Massive code styling
Remove compile warnings
Add samd51 support
@GMagician
Copy link
Contributor Author

GMagician commented Aug 31, 2019

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
Copy link
Member

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
Copy link
Member

thinkyhead commented Aug 31, 2019

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.

@thinkyhead thinkyhead changed the base branch from master to dev August 31, 2019 12:21
@GMagician
Copy link
Contributor Author

GMagician commented Aug 31, 2019

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

edit: and warnings fix?

@thinkyhead
Copy link
Member

thinkyhead commented Aug 31, 2019

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@GMagician GMagician mentioned this pull request Aug 31, 2019
@GMagician
Copy link
Contributor Author

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

@GMagician GMagician closed this Aug 31, 2019
@thinkyhead
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Member

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?

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