dhiltonp / hexbright

The easiest way to start programming your hexbright.
BSD 2-Clause "Simplified" License
124 stars 114 forks source link

New Set_and_Remember program that is a simple modification of up_n_down #51

Closed mcsarge closed 10 years ago

mcsarge commented 10 years ago

Added code to save the light level and keep using that until it is reset. Also added code to detect if the EEPROM has ever been initialized.

dhiltonp commented 10 years ago

The code generally looks good, but there are a couple of things I'd like you to take a second look at:

  1. Indentation - there are a few places where it's hard to follow the code due to non-standard indentation: if(hb.button_pressed_time() >= glow_mode_time && d <= 0.1 && !BIT_CHECK(bitreg,GLOW_MODE_JUST_CHANGED) && !BIT_CHECK(bitreg,QUICKSTROBE) ) { BIT_TOGGLE(bitreg,GLOW_MODE); BIT_SET(bitreg,GLOW_MODE_JUST_CHANGED); }
  2. It may be difficult getting into the lowest brightness without some tweaks to the algorithm: i *= 50; // (0,2000) i -= 5; // (-3,1995) <- something like this i = i<=0 ? 1 : i;

What do you think?

mcsarge commented 10 years ago

I would be glad to make those changes, but I was really trying to not change the original code in up_n_down too much.

There are parts of how things work that I really do not understand yet, hence my hesitation to not change stuff too much from the original.

However, I will go over the code again and get all the indentations correct.

Matt

mcsarge commented 10 years ago

I have updated the formatting and some other things in my copy. Do I need to create a new pull request to get those changes included too?

wbattestilli commented 10 years ago

It's best to make a pull request. If David pulls without one, you won't get credit as a contributor according to github...unless you don't care about such things.

--W.

On Sun, Aug 10, 2014 at 11:07 PM, Matt Sargent notifications@github.com wrote:

I have updated the formatting and some other things in my copy. Do I need to create a new pull request to get those changes included too?

— Reply to this email directly or view it on GitHub https://github.com/dhiltonp/hexbright/pull/51#issuecomment-51738239.

JDWarner commented 10 years ago

I'd like to clarify the process here a bit:

@mcsarge Your changes are already included here; see rev 33699e7 immediately above your prior comment and note the changes are applied if you consult the "Files Changed" tab for the line-by-line diff. GitHub automatically updates PRs with the latest commit you've pushed to the branch this PR originated from on your personal fork of the project.

GitHub will list you as the author of the individual changes to all of these files, even if @dhiltonp or a collaborator merges this PR. They will be listed as the author of the "merge," which officially brought your branch in, but you will be credited in the file history and Git Blame, etc.

No additional PR should be necessary. Any further changes can be applied simply by committing them locally and pushing them up to the branch this PR originated from in your fork of this project.

wbattestilli commented 10 years ago

I stand corrected. I had a situation last year where I was the largest contributor to @dhiltonp's hexbright project and was not listed as a contributor. I historically just told him to merge over IRC but switched to doing pull requests so that I would get credit.

Perhaps something has been fixed in the interim.

On Sun, Aug 10, 2014 at 11:22 PM, Josh Warner notifications@github.com wrote:

I'd like to clarify the process here a bit:

@mcsarge https://github.com/mcsarge Your changes are already included here; see rev 33699e7 https://github.com/dhiltonp/hexbright/commit/33699e722ad4b6bd04d01cbcd21634559595f958 immediately above your comment and note the changes are applied if you consult the "Changes" tab. GitHub automatically updates PRs with the latest commit you've pushed to the branch this PR corresponds to on your personal fork of the project.

GitHub will list you as the author of the individual changes to all of these files, even if @dhiltonp https://github.com/dhiltonp or a collaborator merges this PR. They will be listed as the author of the "merge," which officially brought your branch in, but you will be credited in the file history and Git Blame, etc.

No additional PR should be necessary, and any further changes can be applied simply by committing them locally and pushing them up to the branch this PR corresponds to in your fork of this project.

— Reply to this email directly or view it on GitHub https://github.com/dhiltonp/hexbright/pull/51#issuecomment-51738749.