stereodreieck86/BikeBridge:replay-framework into master
This PR supersedes the previous replay framework PR (#13).
The previous PR did not update correctly after pushing the review changes.
This PR contains:
Just minor changes really. Looks ok overall.
@ -0,0 +1,22 @@
package dev.bg.bikebridge.util.replay
/**
Please can all comments be in English?
@ -0,0 +7,4 @@
* arbitrary String keys in the metadata map.
*/
enum class MetadataKey(val key: String) {
Can whitespace be minimised here?
enum class MetadataKey(val key: String) {
BIKE_MODEL("bike_model"),
MOTOR("motor"),
DISPLAY("display"),
BATTERY("battery"),
REGION("region"),
AUTHOR("author"),
NOTES("notes")
}
I'm ok with it between methods in a class though.
@ -0,0 +143,4 @@
val replay = scenario ?: return null
return ReplayStatistics.calculate(replay)
Same as above in terms of minimising whitespace. It can be like
fun statistics(): ReplayStatisticsResult? {
val replay = scenario ?: return null
return ReplayStatistics.calculate(replay)
}
@ -0,0 +95,4 @@
* Zuerst wird eine temporäre Datei geschrieben.
* Erst danach wird sie in die endgültige Datei umbenannt.
*/
private fun writeAtomically(
I had a comment on the previous PR about this method: #13 (comment)
Do you think this should be renamed?
- Add Bosch helper utilities - Extend BleConstants - Improve BLE logging - Prepare Bosch discovery infrastructure
@bg443 Thanks for the review and sorry for the delay – I got completely absorbed in decoding the Bosch MCSP payloads in the meantime! 😅
I have just pushed an update addressing all your points:
Translated all remaining German KDoc/comments to English across the framework.
Removed the excessive whitespace in MetadataKey and ReplayController.
Renamed writeAtomically to writeSafely in ReplayWriter to make the intent clearer, and translated its documentation.
Let me know if anything else is needed!
Please check whitespace and German comments
@ -0,0 +1,47 @@
# Improve Discovery Logging with Bosch GATT Names
Is this an artifact from an LLM? Do we need it in this PR?
Good catch. Yes, this is an internal planning artifact and was accidentally committed. It is not required for the implementation itself. I'll remove it from this PR to keep the history clean.
@ -178,2 +178,3 @@
with(scanResult) {
device.connectGatt(this@BluetoothGattService, false, gattCallback)
log("========== BLE CONNECT REQUEST ==========")
I'd rather keep == logs out of the log
Makes sense. I'll switch to the existing logging style and remove the separator to keep the logs consistent with the rest of the project.
@ -656,0 +693,4 @@
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
log("Requesting ATT MTU 247")
gatt.requestMtu(247)
Is this consistent across all Bosch bikes?
Most likely YES. More testing is necessry.
@ -682,0 +748,4 @@
log("UUID : ${service.uuid}")
for (characteristic in service.characteristics) {
Please remove unnecessary whitespace
Sure, I'll collapse the simple cases into single-line entries to match the surrounding style.
@ -132,0 +147,4 @@
fun getGattName(uuid: UUID): String =
when (uuid) {
BleConstants.Characteristics.Bosch.LDI_SERVICE ->
nit: one line this BleConstants.Characteristics.Bosch.LDI_SERVICE -> "Bosch LDI Service"
Sure, I'll collapse the simple cases into single-line entries to match the surrounding style.
@ -0,0 +1,36 @@
package dev.bg.bikebridge.util.replay
/**
* Informiert über den Lebenszyklus einer Replay-Wiedergabe.
I still see Deutsch comments :D
Thanks 😄 I missed this one. I'll translate the remaining comments to English for consistency.
Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?