-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Implement initial Kotlin generator #30009
Implement initial Kotlin generator #30009
Conversation
PR #30009: Size comparison from 4823e34 to 059e763 Full report (44 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
PR #30009: Size comparison from 5a284f0 to ee93936 Full report (10 builds for cc32xx, linux, mbed, nrfconnect, qpg)
|
PR #30009: Size comparison from 38e459b to d9a6b91 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have a Super Class or implemented interface for each Cluster Class.
This is not the final version, current cluster APIs does not have any common function to share, we also don't need the chipClusterPtr and devicePtr if we switch to use generic IM API. Will add Super Class later if there is something we could share among all Cluster APIs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@@ -155,6 +155,7 @@ style: | |||
- "**/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/provisioning/EnterNetworkFragment.kt" | |||
- "**/examples/java-matter-controller/java/src/com/matter/controller/commands/common/MatterCommand.kt" | |||
- "**/src/controller/java/src/matter/onboardingpayload/Base38.kt" | |||
- "**/src/controller/java/generated/java/**/*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have kotlin
as prefix instead of java?
@@ -246,6 +247,9 @@ naming: | |||
excludes: | |||
- "**/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/clusterclient/clusterinteraction/HistoryCommandAdapter.kt" | |||
- "**/src/controller/java/generated/java/**/*" | |||
FunctionParameterNaming: | |||
excludes: | |||
- "**/src/controller/java/generated/java/**/*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some doc comments about why this is required for generated code would be nice.
Here is where a separate kotlin directory would be nice too, so we do not inherit leniency from other places - we should try to make even the generated kotlin code look "nice" in as much as possible.
{%- if interfaceName not in already_handled_attribute %} | ||
interface {{interfaceName}} { | ||
fun onSuccess(value: {{encode_value(cluster, encodable, 0)}}) | ||
fun onError(ex: Exception) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onError and onSubscriptionEstablished are always the same. Would it make sense to have a common base interface for them (so that a generic error handler could be written for anything) or would that break some ABI that already exists?
Same for common functions above (e.g. I see on error in other places too)
This is the preliminary implementation of the Kotlin generator. It produces the Kotlin cluster API in a manner compatible with the existing Java Cluster API. This enables us to experiment with using TLV-based generic Im APIs.