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

Any plans to support Generic Delta Set? #573

Open
vkurkchi opened this issue Nov 9, 2023 · 3 comments
Open

Any plans to support Generic Delta Set? #573

vkurkchi opened this issue Nov 9, 2023 · 3 comments

Comments

@vkurkchi
Copy link

vkurkchi commented Nov 9, 2023

After digging into the documentation and the source code, I noticed that the current implementation does not support sending the Generic Delta (Generic Level) messages. The GenericLevelSet message sends the level with only 2 bytes(Mesh Model Spec, 3.2.2.2) without a simple way to morph the level message into delta, while delta messages require 4 bytes(Mesh Model Spec, 3.2.2.4) for the payload.

The solution seems to be pretty straightforward, we can have a class that duplicates the GenericLevelSet interface (let's say GenericLevelDeltaSet) with a different opCode (0x8209), and instead of sending the level with 2 bytes, we can have a delta field of 4 bytes.

Currently, as an alternative, I use a hack like this one:

class GenericLevelDeltaSet(
    appKey: ApplicationKey,
    delta: Int,
    tId: Int,
    transitionSteps: Int? = null,
    transitionResolution: Int? = null,
    delay: Int? = null,
) : GenericUserPropertySet(appKey,
    (delta and 0xFFFF).toShort(),
    if (transitionSteps == null || transitionResolution == null || delay == null) {
        ByteArray(3).apply {
            set(0, (((delta shr 16) and 0xFF).toByte()))
            set(1, (((delta shr 24) and 0xFF).toByte()))
            set(2, ((tId and 0xFF).toByte()))
        }
    } else {
        ByteArray(5).apply{
            set(0, (((delta shr 16) and 0xFF).toByte()))
            set(1, (((delta shr 24) and 0xFF).toByte()))
            set(2, ((tId and 0xFF).toByte()))

            set(3, ((((transitionResolution shl 6) or transitionSteps) and 0xFF).toByte()))
            set(4, ((delay and 0xFF).toByte()))
        }
    }) {
    override fun getOpCode(): Int {
        return 0x8209 // Generic Delta Set
    }
}

In the hack above, I split the delta into "propertyId" and "values" fields.

In general, having a generic application message class for extension would be great (let's say making ApplicationMessage public), that way you can introduce a predictable way for extending the supported messages list.

@roshanrajaratnam
Copy link
Member

Currently I am occupied with other tasks. We are open to PRs :)

@vkurkchi
Copy link
Author

@roshanrajaratnam I've created a PR, also I have one more suggestion, should I create feature request first, or make a PR so you can have a look?

@roshanrajaratnam
Copy link
Member

Hi thanks for the PR, I will have to take a look at this on Monday. Don't forget to sign the CLA on the PR.

Please do create a feature request first so we can take it from there. The reason being this library will go into maintenance mode as we are working on a Kotlin library internally.

Thanks again for your contribution :)

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

No branches or pull requests

2 participants