track firmware version on disconnect

[krushnarout]

When connecting device to app we already track that in Mixpanel—add a parameter for firmware version on that event so >we can see if connection issues correlate with people not updating firmware vs other problems.

we can't track firmware version at connect time because that time firmware version is not guaranteed to be available

[gemini-code-assist]

Code Review

This pull request adds firmware version tracking to the 'Device Disconnected' analytics event. The changes in mixpanel.dart are correct. However, there is a critical issue in device_provider.dart where the logic to retrieve the firmware version is flawed, as it incorrectly inlines fallback logic for a complex case. It doesn't correctly fall back to other sources if the primary source provides an invalid but non-null value like 'Unknown'. I've provided a suggestion to fix this to ensure accurate data tracking, aligning with best practices for handling complex fallbacks.

[gemini-code-assist]

The current logic for retrieving the firmware version might not work as intended. The null-coalescing operator (??) only checks for null, but the firmwareRevision getter can return non-null but invalid values like 'Unknown' or an empty string. If connectedDevice.firmwareRevision is 'Unknown', the subsequent fallbacks to pairedDevice and SharedPreferencesUtil will not be triggered, which could lead to incorrect data being tracked. To ensure you get the first valid firmware version available, you should check for these invalid string values as well. This aligns with the principle of preferring existing helper functions for complex fallback logic to avoid subtle bugs from incorrect inlining.

Suggested change

	final String trackFirmware = connectedDevice?.firmwareRevision ??
	pairedDevice?.firmwareRevision ??
	SharedPreferencesUtil().btDevice.firmwareRevision;
	final String trackFirmware = [connectedDevice?.firmwareRevision, pairedDevice?.firmwareRevision, SharedPreferencesUtil().btDevice.firmwareRevision].firstWhere((v) => v != null && v.isNotEmpty && v != 'Unknown', orElse: () => 'Unknown');

References

1. Prefer using existing helper functions over inlining their logic, especially when they handle complex cases like fallbacks or error handling. Inlining can introduce subtle bugs by missing parts of the original logic.

[beastoin]

firmware version at connect time because that time firmware version is not guaranteed to be available

how did you know ?

[github-actions]

Hey @krushnarout 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

[krushnarout]

how did you know ?

omi/app/lib/providers/device_provider.dart

Line 234 in 651d633

MixpanelManager().deviceConnected();

device connected is tracked before getDeviceInfo() completes, and firmware is fetched asynchronously inside getDeviceInfo(), so firmware is not guaranteed to be available when the connect event sent

[aaravgarg]

how did you know ?

omi/app/lib/providers/device_provider.dart

Line 234 in 651d633

MixpanelManager().deviceConnected();

device connected is tracked before getDeviceInfo() completes, and firmware is fetched asynchronously inside getDeviceInfo(), so firmware is not guaranteed to be available when the connect event sent

@beastoin pls notice

[beastoin]

@krushnarout try harder. when i close your PR, it means either the fix isnt good enough, or your assumptions werent clear.

dont take it personally — make the problem clearer, then come back with a better solution.

[github-actions]

Hey @krushnarout 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

• Project standards — Ensuring consistency across the codebase
• User needs — Making sure changes align with what our users need
• Code best practices — Maintaining code quality and maintainability
• Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

[beastoin]

tips for you: check all calls to getDeviceInfo to get the full picture.

if you use AI, use a bigger model and ask it to trace everything end-to-end — but still validate it yourself. AI helps you move faster; responsibility is still yours.