Change BitmapTransformA-F() methods to accept a single float...

[jamesbowman]

…converting to the appropriate fixed-point internally.

No more fiddly 8.8 / 1.15 fixed-point math in user code.

Confirmed on EVE hardware

[dhalbert]

This is a breaking change to the API. The API is _eve, not eve, so is this somehow considered an internal change, hidden by some library? You are the original submitter of _eve, so you're the expert on this.

I don't really have any idea what the client base of _eve is in CircuitPython.

We generally make breaking changes on a major-version boundary, in this case 10.0, which is not imminent by any means. We often try to make the intermediate versions handle both cases for a while. In this case, the old style takes two arguments, and the new style takes one, so they could be distinguished that way, but it would a noticeable amount of extra code.

[dhalbert]

Side comment re development practices. Instead of submitting PR's from your main branch, we recommend creating a branch for the PR's. That way your main is tied to upstream's main. See https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github/always-work-on-a-branch.

[jamesbowman]

Right, these methods() are visible to the end user: _eve is the base class for eve.

I'll look at a better implementation that's backwards-compatible.

And thanks, a branch per PR is a good idea.

[dhalbert]

thanks - will mark this as "Changes Requested"

[jamesbowman]

The _eve implementation now works in legacy mode by default. This preserves the existing behavior.

When switched into "new" mode it accepts the new argument formats. It wasn't much more code to handle this.

I've added coverage in my test cases for both modes, and it passes on rp2040.

(Note that I had to turn off _eve for the metro_m4_airlift_lite board, as it overflowed the flash. I don't think anyone is using _eve on this tiny board.)

[dhalbert]

The _eve implementation now works in legacy mode by default. This preserves the existing behavior.

When switched into "new" mode it accepts the new argument formats

So, just to clarify my understanding: is the 2-ints-vs-1-float difference a difference in the eve displays themselves, from older to newer versions, or do they all have the same firmware?

I was thinking you could decide which computation to do based on the number of args to each transform function: if two args (ints) are supplied, do it the old way, if one arg (float), do it the new way.

[jamesbowman]

So, just to clarify my understanding: is the 2-ints-vs-1-float difference a difference in the eve displays themselves, from older to newer versions, or do they all have the same firmware?

This is an EVE hardware function, and it works in fixed-point coordinates on all the EVE series. What this change is doing is hiding the fixed-point math, so the user code can use floats.

I was thinking you could decide which computation to do based on the number of args to each transform function: if two args (ints) are supplied, do it the old way, if one arg (float), do it the new way.

Right, I thought of doing this. But two of the methods (BitmapTransformC and BitmapTransformF) take 1 parameter. Either an int in the old style (which represents a signed 15.8 bit fixed-point number) or a float in the new style.

There's also a wrinkle in that the hardware got an upgrade between BT810 and BT815, adding a "high-precision" fixed-point format. The code now deals with this correctly.

[dhalbert]

Alright - thanks!