Introducing the VeryGenericIndicator
#474
Conversation
rafaellehmkuhl
requested review from
patrickelectric,
ES-Alexander and
Williangalvani
rafaellehmkuhl
force-pushed
the
very-generic-indicator
branch
3 times, most recently
from
07788a1 to
a7de368
Compare
rafaellehmkuhl
changed the title
VeryGenericIndicator
There was a problem hiding this comment.
I haven't tested because I didn't want to build, but from what you've shown and discussed it seems like a decent start on our aims. It's super exciting to be able to support arbitrary messages - that's going to be so powerful! 😀
A couple of suggestions for improvement, but nothing critical :-)
| 'id', | ||
| 'gcs_system_id', | ||
| 'sensor_id', | ||
| 'rtk_receiver_id', | ||
| 'compass_id', | ||
| 'gps_id', | ||
| 'storage_id', | ||
| 'camera_id', | ||
| 'stream_id', | ||
| 'gimbal_device_id', | ||
| 'hw_unique_id', | ||
| 'uas_id', |
There was a problem hiding this comment.
Can these be sorted lexicographically/alphabetically? Hard to tell what is and isn't included at the moment. I'm assuming you got all the relevant ones from a search - I only checked the first few in a search and it seems like param_id is missing, but perhaps that was intentional?
Also seems worth including idx and cam_idx from the ardupilotmega message set.
There was a problem hiding this comment.
Will sort alphabetically.
Got them from a search in the MAVLink Common messages page, yes.
param_id was left behind intentionally, as it leads to a lot of garbage variables being registered and (I assume) no valuable data.
Will include idx and cam_idx.
| 'uas_id', | ||
| ] | ||
|
|
||
| const getDeepVariables = (obj: Record<string, unknown>, acc: Record<string, unknown>, baseKey?: string): void => { |
There was a problem hiding this comment.
I think there should be a comment here explaining what this method is for. Maybe something like
/**
* Allows handling messages that are shared by multiple devices, splitting them out by detected device IDs.
*/It may also be worth noting that NAMED_VALUE_* messages are split out via a different function.
| @@ -14,59 +18,79 @@ export interface GenericIndicatorTemplate { | |||
| * Symbols representing the unit system of the variable | |||
| */ | |||
| variableUnit: string | |||
| /** | |||
| * Number of digits to be displayed after the decimal separator (usually dot) | |||
There was a problem hiding this comment.
Is there a setting somewhere that allows people to change the decimal separator, or does it automatically use the separator for the locale of the browser device or something? Would be nice if europeans can use their comma decimal separators, even though they hurt my eyeballs 😂
There was a problem hiding this comment.
🤣
There isn't, and I actually think we shouldn't add it.
We Brazilians also use commas as decimal separators, but this was clearly a big mistake, and softwares like Excel suffer a lot for adding the possibility of changing it, as it causes huge confusions when receiving a spreadsheet.
|
Docs PR here. |
…bject This allows for generic access of any Ardupilot variable from Cockpit users, for example on the `GenericIndicator` widget
With it the user can have a name displayed on the widget that is different from the variable name (which can be cryptic).
…nge with the variable value
There's a lot of situations where the same MAVLink message will bring data from different sources, usually different sensors. We want to separate those as different variables, so the user can choose to display a variable from an specific source, instead of scrambling the data from many. To do so, we look for specific identifier keys in the messages, and use it's values (IDs) to name their variables.
rafaellehmkuhl
force-pushed
the
very-generic-indicator
branch
from
f9baca7 to
0dd448f
Compare
Agree. This opens a lot of doors. Maybe the next cool thing would be to actually store these variables in time, so we can graph them. We should only take care not to blow up memory. |
ES-Alexander
added
docs-complete
Search and include all variables coming from ArduPilot vehicles on the
genericVariablesobject.With this change, we allow the usage of the
GenericIndicatorVeryGenericIndicatormini-widget with any vehicle variable.I'm also adding two new variable to the indicator configuration:
displayName, so the user can select a variable but choose a better name for displaying in the widgetfractionalDigits, so the user can specify how many digits he wants to display after the decimal placeImportant to notice: we are differentiating messages that specify IDs, as suggested by Eliot. For this, I've tried a generic approach, but it leads to many edge-cases, so I just used a list with known identifier keys.
@ES-Alexander could I suggest that you separate the #449 issue into a new issue that lists the suggested improvements (e.g.: color picking, alerts, etc) for the
GenericIndicatormini-widget?Fix #449