ReviewOS

also looking at this

craft-native/craft

feat(android): haptic and vibrate — ratchet 91 -> 89

#150
Merged chris wants to merge feat/android-haptics into main
7 files +410 -3
9 days ago

Ratchet 91 → 89. Fourteen Android actions now served from Zig.

Two SDK branches, and both are live

The Kotlin picks its vibrator two ways — VibratorManager.defaultVibrator from API 31, the deprecated VIBRATOR_SERVICE below it — and its effect two ways, VibrationEffect from 26 and a raw duration below that.

The generator defaults minSdk to 26, so the pre-26 arm looks dead. A caller can pass minSdk: 24 and it isn't. Both are ported rather than assumed away, and the branch is a runtime read of Build.VERSION.SDK_INT exactly as the Kotlin's is.

Worth knowing while reading that arm: it vibrates a flat 20ms whatever the style. On API 24 and 25 every haptic feels identical. That's the shim's behaviour, not a simplification made here.

The style table is millisecond-exact

A page that tuned its feedback around these timings would feel a change that no test of "it vibrated" would catch. Retiming heavy from 50ms to 40ms fails a test.

Matching JSONArray.getLong exactly

It accepts an integer, truncates a double, and parses a numeric string. All three are matched — a page sending ["100"] vibrates on the shim and would stop the day Zig took the action over otherwise.

mutationresult
heavy retimed 50ms → 40msthe style-table test fails
numeric strings refusedthe parse test fails
double rounded instead of truncatedthe parse test fails

A malformed pattern declines rather than erroring: the Kotlin throws, catches, logs "Vibration error" and vibrates nothing. Declining reaches the same silence and keeps the log line, which handling it here would swallow.

One real leak, caught by the testing allocator

parsePattern returned null from inside its loop with the slice already allocated — and errdefer does not cover a null return. Four leaked allocations. The rejections go through an error now, so the guard actually fires.

That's the kind of bug that would never surface in production here, since the caller wraps everything in a per-call arena. It's still wrong in the function's own contract, and the test allocator is the only thing that would ever say so.

JNI

Gained newLongArray, which fills via SetLongArrayRegion in one call rather than element by element — faster, and the only form that can't leave a half-filled array behind on an error.

No table extension was needed. I said last turn this would require extending past 215; that was wrong — NewLongArray (180) and SetLongArrayRegion (212) were already inside the range declared for RegisterNatives.

zig build test:android → 98 tests, 11/11 steps · both .so build · generator 10/10.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.