👋
Can I get a second opinion on https://github.com/home-assistant/core/pull/167612?
#Sensor or Attribute
brave osprey
storm jewel
.
brave osprey
The user is adding four attributes, but I feel that "dateTime" should be a sensor
storm jewel
brave osprey
delay/startDateTime/schedules I think are OK as attributes
It's just "dateTime" (= the last time that the car communicated the data to the server)
Other coordinators have this as a Sensor entity - and I feel it should be the case for this one as well
The user feels differently
storm jewel
Ah, that's always an interesting one as we already track when data got updated
storm jewel
delay/startDateTime/schedules I think are OK as attributes
I don't like schedules though
As in, we already have services for this right
storm jewel
I think we could use one for reading as well
brave osprey
Possibly - but that's another job altogether 😄
I think there is value in a quick-glance at the current data, they are available in the coordinator, and they are marked as _unrecorded_attributes
storm jewel
Like at this point I try to avoid adding attributes as much as possible
lyric zinc
the dateTime seems to be specifically on the charge mode structure, so it may not be the time of the overall data update from the vehicle, but specifically for the charge mode? in that case, I think its fine to be an attribute on the sensor since its directly tied to it
but if we move any of the others out of the attributes, we should also move the dateTime
storm jewel
I think having a date time string in attributes is meh
lyric zinc
why?
storm jewel
It doesn't take timezones and such into account like a timestamp entity does
lyric zinc
true, true
hot pulsar
Feels more like a diagnostic sensor to me...