Skip to content
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

prefer_typed changes the values of strings #256

Closed
Bre77 opened this issue Dec 5, 2024 · 5 comments · May be fixed by #307
Closed

prefer_typed changes the values of strings #256

Bre77 opened this issue Dec 5, 2024 · 5 comments · May be fixed by #307

Comments

@Bre77
Copy link

Bre77 commented Dec 5, 2024

Getting typed values from Fleet Telemetry with prefer_typed is great, but I wanted to check that it was meant to be changing the string values to different string values too.

For example
Sentry Mode has changed:
Off is SentryModeStateOff
Idle is SentryModeStateIdle
Armed is SentryModeStateArmed
etc

These already had pretty good values
, but now the protobuf field names are being returned

@Bre77 Bre77 changed the title prefered_typed changes the values of strings prefer_typed changes the values of strings Dec 5, 2024
@jbanyer
Copy link

jbanyer commented Jan 12, 2025

In addition: not all enum fields are being handled consistently, eg with the JSON format, vehicle version 2024.44.25.4 ad52e3119f65:

With prefer_typed = false:

      {
         "key":"ChargePortLatch",
         "value":{
            "stringValue":"Engaged"
         }
      },
      {
         "key":"DetailedChargeState",
         "value":{
            "detailedChargeStateValue":"DetailedChargeStateCharging"
         }
      },

With prefer_typed = true:

      {
         "key":"ChargePortLatch",
         "value":{
            "chargePortLatchValue":"ChargePortLatchEngaged"
         }
      },
      {
         "key":"DetailedChargeState",
         "value":{
            "detailedChargeStateValue":"DetailedChargeStateCharging"
         }
      },

Notice that DetailedChargeState is handled the same way regardless of prefer_typed, whereas ChargePortLatch is represented differently based on prefer_typed.

Is this going to change with future software releases? What is the recommended approach to handling enum values?

@patrickdemers6
Copy link
Collaborator

When prefer_typed is true, the vehicle sends the data using enums, not strings. When converting to JSON, UseEnumNumbers is currently hardcoded to false.

jsonOptions = protojson.MarshalOptions{
UseEnumNumbers: false,
EmitUnpopulated: true,
Indent: ""}

I will work on better documenting this, but don't think server-side changes are ideal as:

  • This would be breaking
  • Only those migrating to prefer_typed:true and using transmit_decoded_records:true are impacted.

DetailedChargeState is always returning DetailedChargeStateCharging since it is marked as force typed on the vehicle. All fields that are newly added are forced to be the typed versions, instead of strings.

@rawmean
Copy link

rawmean commented Jan 16, 2025

@patrickdemers6 Thank you for your hard work. Can you please document the effect of these fields a little more formally? AFAIK, there is zero documentation for prefer_types while using minimum_data requires it.

If I understand your comment above correctly, the format changes depending on values of both prefer_typed and transmit_decoded_records fields (which are undocumented AFAIK).

Migration becomes a headache when there are inconsistencies like the ones that are reported by @jbanyer and @Bre77 reported above.

Please note that we have to deal with migrations on the client side as well and changes like this will force us to run two instances of the server while clients (eg, mobiles apps) are being updated gradually and vehicles get the new firmware updates.

Also testing of these new features is not possible for those of us who do not have the newer firmware (2024.45.32 has been installed on a small fraction of cars). One option would be for developer to register a couple of VINs to get these updates sooner to be able to test.

@rawmean
Copy link

rawmean commented Jan 16, 2025

Is it not possible to change to photo file such that the enums are consistent and the problem that @Bre77 and @jbanyer reported above not occur?

@Bre77
Copy link
Author

Bre77 commented Jan 16, 2025

Is it not possible to change to photo file such that the enums are consistent and the problem that @Bre77 and @jbanyer reported above not occur?

I'm happy with Patrick's response and it makes complete sense.

Due to the consistent naming of the Enums, I'm handing it fairly easily in my client.
https://github.com/Teslemetry/python-teslemetry-stream/blob/92ec8c3ed8290598bca8ac120721bdb95ff5f0d0/teslemetry_stream/const.py#L270

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants