update quantum/template/avr/config.h

[mtei]

Description

Modify the template of config.h according to drashna's recommendation. And I think the RGBLIGHT_ANIMATIONS is only for backwards compatibility. I think its use should be deprecated. So I removed it.

see #8670 (comment)

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[fauxpark]

The LTO block should also use proper formatting.

[zvecr]

Has there been discussion to deprecation of RGBLIGHT_ANIMATIONS? Given it also functions as "give me all available animations", i'm not sure its only for backwards compatibility.

[mtei]

The LTO block should also use proper formatting.

I didn't change it to avoid conflicting with #8663.

[mtei]

Has there been discussion to deprecation of RGBLIGHT_ANIMATIONS?

I wanted to discuss that here, so I made this change.

When I made rgblight.c configurable (#3582), RGBLIGHT_ANIMATIONS was referenced in several source files under quantum/ and tmk_core/, so I didn't remove it for fear of negative consequences.

Later, due to refactoring by you and others, RGBLIGHT_ANIMATIONS is no longer referenced under quantum/ and tmk_core/.

Currently, RGBLIGHT_ANIMATIONS is only there to ensure backward compatibility for keymap users. Of course, this compatibility should be maintained in the future.

But with the new keyboard, I think it's better not to use RGBLIGHT_ANIMATIONS, but to choose the effect you want to use explicitly. So I propose to remove it from the template.

If, after discussion, we come to the conclusion that we want to keep RGBLIGHT_ANIMATIONS, I will add it again to the template.

[mtei]

Given it also functions as "give me all available animations", i'm not sure its only for backwards compatibility.

If someone adds a new effect to rgblight.c in the future, it will automatically increase the firmware size of the keyboard using RGBLIGHT_ANIMATIONS.

Is this an acceptable thing to do?
Is adding a new effect a breaking change?

So, I don't think RGBLIGHT_ANIMATIONS should be used on new keyboards. And when you add a new effect, you shouldn't include it in RGBLIGHT_ANIMATIONS.

[zvecr]

So I agree we have space limitations on AVR boards, and that factors partially into the decision.

However for end users, having RGB_MOD cycle through everything we have is far more useful that having to enable each one, potentially in turn, to see which one you like. I would say disabling other features like console is a worthwhile trade off to the increase in firmware space due to animations.

[mtei]

This PR template has all RGBlight effects enabled. It's easy for users to see which one they like. I think the reasons you gave are irrelevant to the question of whether or not to leave RGBLIGHT_ANIMATIONS in the template.

And with RGBLIGHT_ANIMATIONS, you'll be affected when RGBlight effects are added in the future, as mentioned above. When a user brings qmk_fiermware up to date, it may fail to compile due to an unexpected increase in firmware size. Therefore, it is better not to use RGBLIGHT_ANIMATIONS on new keyboards to avoid that risk.

[zvecr]

I disagree that they are irrelevant, and would put forward that your case is weak for the justification for removal.

While I haven't requested any changes, this is me formally flagging that I disagree with the proposed direction. And suggest we get other collabs to sign off so it has a net approval of at least +1.

[fauxpark]

Looks like the "backward compatibility" notice was added in #7773?

[mtei]

Looks like the "backward compatibility" notice was added in #7773?

Which notice are you referring to?

[fauxpark]

Ah, never mind, it came from rgblight_reconfig.h in 3582.

[stale]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[stale]

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.