jasoncoon / esp8266-fastled-webserver

GNU General Public License v3.0
712 stars 360 forks source link

map.cpp - Fix regression for "up" direction, as used by clocks #225

Closed henrygab closed 2 years ago

henrygab commented 2 years ago

This is to capture the discussion regarding which way is defaulting to "up" on each board. Specifically, as part of PR #213, "up" was defined to use 64u - hour (etc.). See commit 0702b2664362d354b25e28da9d4d2b874aba383b, file map.cpp.

However, IIRC, the original branches used different values for "up" when defining the clock's location for 3/6/9/12. The issue tracks the review of the following two functions, back to the original (per-product) branches:

The branches in question are:

As can be see, every branch calculated "up" as 255 - hour, with the exception of Fibonacci256. Only Fibonacci256 calculated it as the currently-checked-in code does (using 64u - hour). As an aside, it would likely be more correct to use 256u - hour ... casting to uint8_t to cause angle to remain in range [0..255] ... for the other products.

This leaves only one question:

henrygab commented 2 years ago

@jasoncoon -- I can see two simple ways to fix this.

  1. rotate the positions / angles of Fibonacci256, so that up is at zero degrees (256u ... matching all others)

  2. define new symbol WHICH_WAY_IS_UP for configuration, used only by Fibonacci256

I'm leaning towards the first option. It would entail the following:

A. Subtract 64 from the values stored in angles B. Rotate values stored in both coordsX and coordsY counter-clockwise 90 degrees. Should be as simple as (untested):

Which would you prefer?

jasoncoon commented 2 years ago

I've confirmed the current 64 - hour works for all of the Fibonacci PCBs.

I should enable the clock patterns for 1628 rings, but they probably don't make any sense for kraken and chamaeleon.

henrygab commented 2 years ago

Ok, I'm fine either way... just wanted to ensure it was a conscious decision, and not an accidental regression.

Note that this will change behavior for existing boards that upgrade, by effectively rotating the output 90 degrees. I'll add to the wiki.