-
-
Notifications
You must be signed in to change notification settings - Fork 60
Add longpressthreshold setting to the chameleon settings page #565
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,10 @@ enum ChameleonCommand { | |
bleGetPairEnable(1036), | ||
bleSetPairEnable(1037), | ||
|
||
// long press threshold | ||
getLongPressThreshold(1038), | ||
setLongPressThreshold(1039), | ||
|
||
// hf reader commands | ||
scan14ATag(2000), | ||
mf1SupportDetect(2001), | ||
|
@@ -344,6 +348,7 @@ class DeviceSettings { | |
ButtonConfig bLongPress; | ||
bool pairingEnabled; | ||
String key; | ||
int longPressThreshold; | ||
|
||
DeviceSettings( | ||
{this.animation = AnimationSetting.none, | ||
|
@@ -352,7 +357,8 @@ class DeviceSettings { | |
this.aLongPress = ButtonConfig.disable, | ||
this.bLongPress = ButtonConfig.disable, | ||
this.pairingEnabled = false, | ||
this.key = ""}); | ||
this.key = "", | ||
this.longPressThreshold = 1000}); | ||
} | ||
|
||
enum MifareClassicValueBlockOperator { | ||
|
@@ -1147,7 +1153,7 @@ class ChameleonCommunicator { | |
|
||
Future<DeviceSettings> getDeviceSettings() async { | ||
var resp = (await sendCmd(ChameleonCommand.getDeviceSettings))!.data; | ||
if (resp[0] != 5) { | ||
if (resp[0] != 5 && resp[0] != 6) { | ||
throw ("Invalid settings version"); | ||
} | ||
|
||
|
@@ -1157,14 +1163,23 @@ class ChameleonCommunicator { | |
aLongPress = getButtonConfigType(resp[4]), | ||
bLongPress = getButtonConfigType(resp[5]); | ||
|
||
// Default long press threshold is 1000ms | ||
int longPressThreshold = 1000; | ||
|
||
// Version 6 includes the long press threshold | ||
if (resp[0] == 6 && resp.length >= 15) { | ||
longPressThreshold = bytesToU16(resp.sublist(13, 15)); | ||
} | ||
|
||
return DeviceSettings( | ||
animation: animationMode, | ||
aPress: aPress, | ||
bPress: bPress, | ||
aLongPress: aLongPress, | ||
bLongPress: bLongPress, | ||
pairingEnabled: resp[6] == 1, | ||
key: utf8.decode(resp.sublist(7, 13), allowMalformed: true)); | ||
key: utf8.decode(resp.sublist(7, 13), allowMalformed: true), | ||
longPressThreshold: longPressThreshold); | ||
} | ||
|
||
Future<List<int>> getDeviceCapabilities() async { | ||
|
@@ -1308,4 +1323,30 @@ class ChameleonCommunicator { | |
value & 0xFF | ||
])); | ||
} | ||
|
||
Future<int> getLongPressThreshold() async { | ||
var resp = await sendCmd(ChameleonCommand.getLongPressThreshold); | ||
if (resp!.data.length != 2) throw ("Invalid data length"); | ||
return bytesToU16(resp.data); | ||
} | ||
|
||
Future<bool> setLongPressThreshold(int threshold) async { | ||
try { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not catch exceptions in bridge code, you should catch them inside app code |
||
// Enforce minimum value of 200ms | ||
if (threshold < 200) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same there, should be capped on the app code, not library There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know it's enforced by the firmware, but the user should get some feedback, not just set it to 50 only for it to either magically turn into 200 or stay the same. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Msprg "should be capped on the app code" means, that it should be capped on the app code, not that it shouldnt be capped. Have a slider or smthng and make the UI (app) code limit it. allows you to also give more descriptive error messages |
||
threshold = 200; | ||
} | ||
// Maximum value is implicitly handled by uint16 (65535) | ||
if (threshold > 65535) { | ||
threshold = 65535; | ||
} | ||
|
||
await sendCmd(ChameleonCommand.setLongPressThreshold, | ||
data: Uint8List.fromList(u16ToBytes(threshold))); | ||
return true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't check real return, should return |
||
} catch (e) { | ||
log.e("Error setting long press threshold: $e"); | ||
return false; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,9 +18,50 @@ class ChameleonSettings extends StatefulWidget { | |
} | ||
|
||
class ChameleonSettingsState extends State<ChameleonSettings> { | ||
int? currentLongPressThreshold; | ||
|
||
@override | ||
void initState() { | ||
super.initState(); | ||
// Fetch the current long press threshold when the widget initializes | ||
_updateLongPressThreshold(); | ||
} | ||
|
||
Future<void> _updateLongPressThreshold() async { | ||
if (!mounted) return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just no, there is |
||
|
||
var appState = context.read<ChameleonGUIState>(); | ||
|
||
// Don't attempt to fetch if not connected | ||
if (!appState.connector!.connected || appState.communicator == null) return; | ||
|
||
try { | ||
int threshold = await appState.communicator!.getLongPressThreshold(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And well, getDeviceSettings returns threshold... |
||
if (mounted) { | ||
setState(() { | ||
currentLongPressThreshold = threshold; | ||
}); | ||
} | ||
} catch (e) { | ||
// Log the error but don't display it to the user, we'll use the default value | ||
appState.log?.w("Error getting long press threshold: $e"); | ||
|
||
// If this was a timeout or busy error, try again once after a short delay | ||
if (e.toString().contains("Timeout") && mounted) { | ||
await Future.delayed(const Duration(milliseconds: 500)); | ||
try { | ||
int threshold = await appState.communicator!.getLongPressThreshold(); | ||
if (mounted) { | ||
setState(() { | ||
currentLongPressThreshold = threshold; | ||
}); | ||
} | ||
} catch (retryError) { | ||
// Just use the default from settings | ||
appState.log?.w("Retry failed: $retryError"); | ||
} | ||
} | ||
} | ||
} | ||
|
||
Future<DeviceSettings> getSettingsData() async { | ||
|
@@ -291,6 +332,113 @@ class ChameleonSettingsState extends State<ChameleonSettings> { | |
appState.changesMade(); | ||
}), | ||
const SizedBox(height: 10), | ||
Text("Buttons Long Press Threshold:", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create localization |
||
textScaler: const TextScaler.linear(0.9)), | ||
const SizedBox(height: 7), | ||
Builder( | ||
builder: (context) { | ||
// Use the directly fetched value if available, otherwise fall back to settings | ||
final threshold = currentLongPressThreshold ?? settings.longPressThreshold; | ||
|
||
// Create the controller outside of setState to persist the value | ||
final TextEditingController thresholdController = | ||
TextEditingController(text: threshold.toString()); | ||
final GlobalKey<FormFieldState> fieldKey = GlobalKey<FormFieldState>(); | ||
|
||
return Row( | ||
children: [ | ||
Expanded( | ||
child: TextFormField( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slider should be better there, I doubt someone intentionally want to set threshold to exactly 5032 ms There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I dislike the slider as it offers either low precision and / or low range. I like to fine tune my values, and while I agree 5032ms is just stupid, values such as 210, 215, 225, 250, will be difficult to set. It could be supplemented by a slider, but not replaced. It's also good for accessibility. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Msprg have the classic arrangement of a slider next to a small textbox. best of both worlds. Maybe have the slider snap to some reasonable values (default value in the middle), i think that should be possible. |
||
key: fieldKey, | ||
controller: thresholdController, | ||
keyboardType: TextInputType.number, | ||
decoration: const InputDecoration( | ||
labelText: "Threshold (ms)", | ||
helperText: "200-65535 ms", | ||
hintText: "1000", | ||
), | ||
validator: (value) { | ||
if (value == null || value.isEmpty) { | ||
return "Please enter a value"; | ||
} | ||
int? threshold = int.tryParse(value); | ||
if (threshold == null) { | ||
return "Please enter a valid number"; | ||
} | ||
if (threshold < 200) { | ||
return "Minimum is 200ms"; | ||
} | ||
if (threshold > 65535) { | ||
return "Maximum is 65535ms"; | ||
} | ||
return null; | ||
}, | ||
), | ||
), | ||
ElevatedButton( | ||
child: const Text("Save"), | ||
onPressed: () async { | ||
if (fieldKey.currentState!.validate()) { | ||
try { | ||
int threshold = int.parse(thresholdController.text); | ||
bool success = await appState.communicator!.setLongPressThreshold(threshold); | ||
|
||
if (success) { | ||
await appState.communicator!.saveSettings(); | ||
|
||
setState(() { | ||
currentLongPressThreshold = threshold; | ||
}); | ||
|
||
appState.changesMade(); | ||
|
||
} else { | ||
ScaffoldMessenger.of(context).showSnackBar( | ||
SnackBar( | ||
content: const Text("Failed to set threshold"), | ||
backgroundColor: Colors.red, | ||
), | ||
); | ||
} | ||
} catch (e) { | ||
ScaffoldMessenger.of(context).showSnackBar( | ||
SnackBar( | ||
content: Text("Error saving threshold: ${e.toString()}"), | ||
backgroundColor: Colors.red, | ||
), | ||
); | ||
|
||
if (!appState.connector!.connected) { | ||
Navigator.of(context).pop(); | ||
} | ||
} | ||
} | ||
}, | ||
), | ||
IconButton( | ||
icon: const Icon(Icons.info_outline), | ||
onPressed: () { | ||
showDialog( | ||
context: context, | ||
builder: (BuildContext context) => AlertDialog( | ||
title: const Text("Long Press Threshold"), | ||
content: const Text( | ||
"Sets the time in milliseconds for how long a button needs to be pressed to be considered a 'long press'.\n\nDefault: 1000ms (1 second)"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Long press threshold" is self explanatory |
||
actions: <Widget>[ | ||
TextButton( | ||
onPressed: () => Navigator.pop(context), | ||
child: Text(localizations.close), | ||
), | ||
], | ||
), | ||
); | ||
}, | ||
), | ||
], | ||
); | ||
} | ||
), | ||
const SizedBox(height: 10), | ||
const Text("BLE:"), | ||
const SizedBox(height: 10), | ||
Text('${localizations.ble_pairing}:'), | ||
|
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.
Return null if not possible to fetch