Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Level Control Cluster persists all values during move-to-level #23222

Closed
markus-becker-tridonic-com opened this issue Oct 17, 2022 · 2 comments · Fixed by #23366
Closed

Comments

@markus-becker-tridonic-com
Copy link
Contributor

Reproduction steps

1. Read persistence storage before level-control move-to-level
2. Perform dimming via level-control move-to-level
3. Read persistence storage after level-control move-to-level

All intermediate values are persisted.

This poses 4 problems:
* Flash wear-out and hard to estimate life-time of a device based on this behaviour.
* Timing of level-control due to write and possibly erase of flash (possibly dependent on platform implementation)
* If a device is switched off and turned on, all other devices doing the dimming will likely have reached the final value, while the device being switched off and turned on will be at an intermediate value and not continue dimming to the final value.
* If a device is automatically controlled (e.g. by illuminance measurements) a device will constantly dim and wear out the flash even faster.

Proposal:
Introduce a new ZAP storage option, e.g. NVM deferred, that delays storage of frequently changed values with a certain duration until the value has stabilized.

Bug prevalence

Every time level-control current-level is changed

GitHub hash of the SDK that was being used

v1.0

Platform

nrf, other

Platform Version(s)

NCS2.0.2

Anything else?

No response

@Damian-Nordic
Copy link
Contributor

I can imagine solutions at two levels:

  1. Handle it at the application level by inheriting from DefaultAttributePersistenceProvider and overriding its WriteValue method as presented in the pseudocode below
CHIP_ERROR MyAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath,
                                                           const EmberAfAttributeMetadata * aMetadata, const ByteSpan & aValue)
{
    if (IsDeferredWriteAttribute(aPath))
    {
        if (AddOrUpdateWriteBuffer(aPath, aValue) == CHIP_NO_ERROR)
        {
            StartTimer(1000ms, FlushWriteBuffer);
            return CHIP_NO_ERROR;
        }
    }

    return DefaultAttributePersistenceProvider::WriteValue(aPath, aMetadata, aValue);
}
  1. Handle that at the Matter stack level by introducing another storage type, e.g. NVM Deferred, and provide a standard AttributePersistenceProvider decorator class that handles deferred writes.

@bzbarsky-apple
Copy link
Contributor

The app level solution is the right one for now. Figuring out a batching/deferral strategy for NVM attributes is something that did not happen for 1.0 but might happen as we do follow-up work; it was not clear whether there is a reasonable default strategy that would work for most cases well (because any deferral strategy runs the risk of data loss, etc, and the tradeoffs there might be very application-specific).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants