-
-
Notifications
You must be signed in to change notification settings - Fork 380
refactor: Migrate from Protobuf-Java to Wire #4246
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4246 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 3 3
Lines 23 23
Branches 7 7
=====================================
Misses 23 23 ☔ View full report in Codecov by Sentry. |
This commit migrates the project from using the `protobuf-java` library to `wire`. This involves extensive changes across the codebase to adapt to the new generated models and their APIs. Key changes include: - Replaced Protobuf builders with Wire's immutable data classes and `copy()` method. - Updated field access from camelCase (`longName`) to snake_case (`long_name`). - Switched from `com.google.protobuf.ByteString` to `okio.ByteString`. - Replaced Protobuf's `hasField()` checks with null checks on Wire's optional fields. - Adapted `enum` handling to work with Wire-generated enums, removing "UNRECOGNIZED" filters. - Removed Protobuf-specific utility code, such as `userFieldsToString` and `compareUsers` which relied on Descriptors. - Updated Gradle build scripts (`build.gradle.kts`, `libs.versions.toml`) to remove Protobuf dependencies and add Wire dependencies and the Wire plugin. - Updated ProGuard rules to keep Wire-generated classes. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit migrates the codebase to the newly generated protobuf bindings.
Key changes include:
- Replacing builder functions (e.g., `channelSet {}`) with direct constructor calls (e.g., `ChannelSet()`).
- Updating references from `Portnums.PortNum.X_VALUE` to `PortNum.X.value`.
- Changing field access from `snake_case` to `camelCase` where applicable (e.g., `myInfo.pio_env` to `myInfo.pioEnv`).
- Removing an unused `clearMyNodeInfo` method from `NodeRepository`.
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
When adding channels from a QR code, existing channels were not being correctly preserved. This change refactors the logic to ensure that existing channels are always kept. The UI is also updated to disable selection for existing channels, making them implicitly included and preventing users from deselecting them. Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
This commit removes a significant number of unsafe nullable assertions (`!!` and `?:`) across the codebase, replacing them with safe calls (`?.`), Elvis operators (`?:`), and explicit checks. It aligns the code with Wire's generated non-nullable fields, improving stability and reducing NullPointerException risks.
### Key Changes:
- **Null Safety:** Replaced unsafe non-null assertions on protobuf message fields (e.g., `hop_limit!!`, `channel_num ?: 0`, `bytes ?: ByteArray(0)`) with safe access patterns. This is prominent in `MeshDataHandler`, `MeshCommandSender`, `LoRaConfigItemList`, and various UI components.
- **Wire Protocol Buffers:**
- Refactored `MeshUser` into a dedicated model class, separate from the proto-generated `User` class, with `toProto()` conversion. Introduced `UserExtensions.kt` for cleaner access to `User` fields.
- Added golden file tests for `MeshPacket`, `Config`, and `User` protobuf messages to verify serialization and prevent regressions.
- **Metric Handling:**
- Updated environmental and power metrics display logic to handle `NaN` values correctly, preventing them from being shown as `0` or causing crashes. Zero is now treated as a valid value for temperature.
- `MetricsViewModel` now correctly respects the global `IMPERIAL` display unit for showing temperature in Fahrenheit, in addition to the specific module setting.
- **Configuration & State Management:**
- `MeshConfigHandler` now dynamically calculates the number of config/module fields instead of using hardcoded values.
- It also properly tracks the loading state for configs and channels, preventing `MeshService` from returning empty defaults on timeout by throwing an `IllegalStateException` instead.
- **Hop Limit Default:**
- `MeshCommandSender` now enforces a default hop limit of `3` if the device configuration has a hop limit of `0`, preventing packets from being dropped.
- **Build & Test:**
- Enabled Android resources in unit tests for the `feature/node` module and added Robolectric.
- Added new unit tests for hop limit logic, telemetry parsing, and UI components.
Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>
| data class DataPacket( | ||
| var to: String? = ID_BROADCAST, // a nodeID string, or ID_BROADCAST for broadcast | ||
| val bytes: ByteArray?, | ||
| var bytes: ByteArray?, |
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.
What about continuing the Okio types to here?
@Serializable(with = ByteStringSerializer::class)
@TypeParceler<ByteString?, ByteStringParceler>
var bytes: ByteString?,
and
object ByteStringSerializer : KSerializer<ByteString> {
val byteArraySerializer = ByteArraySerializer()
override val descriptor: SerialDescriptor = byteArraySerializer.descriptor
override fun serialize(encoder: Encoder, value: ByteString) {
byteArraySerializer.serialize(encoder, value.toByteArray())
}
override fun deserialize(decoder: Decoder): ByteString {
return byteArraySerializer.deserialize(decoder).toByteString()
}
}
object ByteStringParceler : Parceler<ByteString?> {
override fun create(parcel: Parcel): ByteString? {
return parcel.createByteArray()?.toByteString()
}
override fun ByteString?.write(parcel: Parcel, flags: Int) {
parcel.writeByteArray(this?.toByteArray())
}
}
| ) | ||
|
|
||
| @Suppress("CyclomaticComplexMethod") | ||
| override fun equals(other: Any?): Boolean { |
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.
If you use ByteString for the type, I think you can delete equals/hashCode. And maybe readFromParcel.
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.
readFromParcel might be needed for the inout param on send.
This commit migrates the project from using the
protobuf-javalibrary towire. This involves extensive changes across the codebase to adapt to the new generated models and their APIs.Key changes include:
copy()method.longName) to snake_case (long_name).com.google.protobuf.ByteStringtookio.ByteString.hasField()checks with null checks on Wire's optional fields.enumhandling to work with Wire-generated enums, removing "UNRECOGNIZED" filters.userFieldsToStringandcompareUserswhich relied on Descriptors.build.gradle.kts,libs.versions.toml) to remove Protobuf dependencies and add Wire dependencies and the Wire plugin.