avr-rust / delay

arduino-like delay routines based on busy-wait loops
Apache License 2.0
15 stars 11 forks source link

Commit messages of the rewrite #22

Closed stappersg closed 2 years ago

stappersg commented 2 years ago

In #17 are many things discussed. None of that information is in branch rewrite. That shouldn't be.

Have look at https://github.com/ipxe/ipxe/commits for examples of good commit messages.

Thing I'm asking for is commit messages, not only commit summaries.

Rule of thumb: More than one line.

Please provide the missing messages for branch rewrite.

lord-ne commented 2 years ago

How do I add the messages to commits that already exist?

stappersg commented 2 years ago

On Thu, Jun 09, 2022 at 03:53:00PM -0700, lord-ne wrote:

How do I add the messages to commits that already exist?

I can do that. (It will be done outside "github.com".)

It is the text that this issue should yield. Paste them here.

Groeten Geert Stappers -- Silence is hard to parse

lord-ne commented 2 years ago

Here is the commit message for the first commit (Add delay_count and delay_loop internal functions):

Move delay loop into internal functions

Refactor the assembly loop into its own function, to allow easier
reuse in the below functions. 

Add internal delay functions calling the assembly loop function.
One function uses a 32 bit counter, and the other uses a 48 bit counter.
These functions will be used to reactor delay_ms and delay_us to prevent
overflow. 

Refactor delay to use the internal delay functions.

Here is the commit message for the second commit (Implement delay, delay_ms, and delay_us):

Refactor delay_ms and delay_us to prevent overflow

Change delay_ms and delay_us to call the internal delay function with
a 48 bit counter. This prevents delay_ms and delay_us from overflowing
when passed a large argument.

Move the internal delay functions to their own module. Remove the
unused internal delay_loop_3_cycles function.

Here is the commit message for the third commit (Divide numerator and denominator by their GCD at compile time):

Divide numerator and denominator by their GCD at compile time

Calculating the number of cycles to delay for in delay_ms and delay_us
requires multiplying by a constant and dividing by another constant. Divide
both constants by their GCD at compile time, to allow the compiler to
better optimize the multiplication and division. This reduces the overhead in
the case where the compiler does not know the length of the delay at
compile time, or otherwise fails to inline the function.
stappersg commented 2 years ago

Those commit message are now at http://uta.stappers.it/git/avr-rust/delay in branch rerewrite use something like

git remote add stappers http://uta.stappers.it/git/avr-rust/delay
git fetch stappers
git checkout rerewrite 

to get them. File src/lib.rs now still contains

#[cfg(test)]
mod tests {
    #[test]
    fn it_works() {
        assert_eq!(2 + 2, 4);
    }
}

For those who like to remove that #[cfg(test)], ask for it a separate merge request. Thing I'm really telling is that I don't like silent removal of code. Addtional note: no hard feelings. :-)

@lord-ne if you want the "rewrite changes", that originate from #17, merged, make a merge request so we can discuss this further. Branch rerewrite is created for that purpose.

lord-ne commented 2 years ago

I created the pull request, so I think this issue can be closed now

stappersg commented 2 years ago

merged (now closing the issue)