Changing setFaceColor to setColorOnFace, want to send a deprecation warning

Software Resources
1

So in using the Blinks library, I’ve run across a little grammatical annoyance. Right now when you send a value on a specific face, you use setValueSentOnFace(value, face). This makes sense because in the wording of the function, value comes first and face comes second. It also has to be this way because setFaceValue is slightly ambiguous, since values are both sent and received.

For color, it’s opposite. The function is setFaceColor(face, color), which, while internally consistent, is backwards from how values are sent. I’m proposing that we change this function to setColorOnFace(color, face) so that the two functions are parallel, which I think will lead to fewer accidents and a sense of consistency.

All that aside, I was very quickly able to make setColorOnFace a function in the library by adding it to both blinklib.h and blinklib.cpp. I literally just copied the code for the old function and added a new one beneath it with identical functionality, and it worked just fine on my blink.

Now that I’ve done that, I’d like to know if there was a way to send a deprecation warning when a developer uses the “old” version of setFaceColor. Something like “You are using setFaceColor(face, color), which is being deprecated. Please use setColorOnFace(color, face) in its place.”

Does anyone know how to add warnings to the codebase?

1 Like
2

So once again I’ve solved my own problem by posting THEN googling, but I think it’s still useful information for anyone reading this.

Adding a warning was super easy. Inside the function in blinklib.cpp I just did this:

void setFaceColor( byte face , Color newColor ) {

    pixelColor_t newPixelColor;

    // TODO: OMG, this is the most inefficient conversion from a unit16 back to (the same) unit16 ever!
    // But to share a type between the core and blinklib level though pixel.h would require all blinklib
    // users to get the whole pixel.h namespace. There has to be a good way around this. Maybe
    // break out the pixelColor type into its own core .H file? seems wrong. Hmmm....

    newPixelColor.r = GET_5BIT_R( newColor );
    newPixelColor.g = GET_5BIT_G( newColor );
    newPixelColor.b = GET_5BIT_B( newColor );

    pixel_bufferedSetPixel( face , newPixelColor );
	#warning You are using setFaceColor(face, color), which is being deprecated. Please use setColorOnFace(color, face) in its place.
}

Here’s what I discovered, though. In order to see this warning, you must have compiler warnings set to ALL in preferences.

preferences
preferences.png708×312 7.87 KB

Hope this is helpful to someone (besides me).

3

Yes, this reverse parameter across methods bit me too. I put in a request for consistency in the API and API doc awhile ago. I’m fine with your proposed naming convention.

4

Looks like @danking222 will drop this into a pull request soon to be added to the current API. @kenj let us know if you have any questions regarding process for submitting pull requests as well.