[renault] Renault more channels and HVAC ON / toggle charge mode

[dougculnane]

The initial Renault binding added in openhab 3.2 had a few basic channels. This update adds more channels and the ability to interact with the car.

Turning the HVAC ON allows users to pre-heat their cars.

Switching charge mode allows users to control the charge limit of their cars without a smart charger.

There is much discussion, testing and feedback to these changes in the forum starting here: https://community.openhab.org/t/betatest-renault-ze-services-binding/72226/127

@lolodomo kindly reviewed inital contribution. I have tried to continue the style of his feedback with the additional changes.

The HVAC ON command to the car and wait for a response from the server is a problem i have tried to solve in a not too confusing way. I hope this is OK. The status is only polled every 10 minutes so feedback that the HVAC is really ON needs a delayed response to give the car and MyRenault server time to react....

[lolodomo]

I will review this PR.

[dougculnane]

@lolodomo Thank you for reviewing this update to the binding.

The HVAC ON command to the car and wait for a response from the server might not be the most openHAB way to do this. If you know of a better technique please let me know.

[dougculnane]

@lolodomo The binding is working well (again). However there are some strange Renault API behaviours that I do not fully understand.

I will do the mvn i18n:generate-default-translations command and get the German and English translations done soon.

I am still not sure how to avoid generic Exceptions without a massive catch list or a dead job task...

[lolodomo]

I will do the mvn i18n:generate-default-translations command and get the German and English translations done soon.

The German translation must not be part of this PR. It should be done in Crowdin.

I am still not sure how to avoid generic Exceptions without a massive catch list or a dead job task...

Let me check again, if you are in the case of scheduled task, this may be acceptable.

[lolodomo]

Let me check again, if you are in the case of scheduled task, this may be acceptable.

This is not code run in a regular scheduled task so I see no reason to not catch so largely.

[dougculnane]

@lolodomo I have pushed a list of errors that could be thrown. I am not sure if this is what you wanted me to do. I am concerned that a runtime error (like "SSL cert no long valid", or "Unknown host exception".... ) will throw a nasty error up the openHAB stack to be handled elsewhere. I am not sure I can for-see all the unforeseen situations.... if I try to list them. Maybe I misunderstand this or maybe openHAB handles these situations better. Please let me know what you are after if my push is not it.

[dougculnane]

@lolodomo I have a bug report here: https://community.openhab.org/t/betatest-renault-ze-services-binding/72226/176?u=doug_culnane

Basically it is suggested that Number:Lengths should be set using meter values and not the KM value. Is this maybe a binding issue I should change or an openHAB bug that some components assume meters?

[lolodomo]

You have to check / log the value returned by the API. Is this value in meters or kilometers. Your code assumes it is in kilometers.
If you have already something in your code allowing to log the API response, ask the user to provide this log to you.

[dougculnane]

@lolodomo Thanks for your response. I can reproduce the bug I think and I think it is an "analysis screen" issue.

https://community.openhab.org/t/betatest-renault-ze-services-binding/72226/180?u=doug_culnane

Therefore I will not try to patch the binding at this stage.

[lolodomo]

Thanks for your response. I can reproduce the bug I think and I think it is an "analysis screen" issue.

What is important for you is the state value of the linked item.

[dougculnane]

What is important for you is the state value of the linked item.

This is good I have check the display value and it is correct and the mysql stored value which is also correct (in meters)

| 2022-02-10 13:17:45 | 7642230 |
| 2022-02-10 16:28:39 | 7642540 |
| 2022-02-10 16:58:49 | 7647060 |
| 2022-02-10 17:08:53 | 7650330 |
| 2022-02-10 17:18:57 | 7653790 |
+---------------------+---------+
258 rows in set (0.001 sec)

MariaDB [openhab]> select * from item0989;

[lolodomo]

This is good I have check the display value and it is correct and the mysql stored value which is also correct (in meters)

Why in meters ? Your code assumes that the API value is in kilometers if you provide unit KILO(METER) when building the QuantityType.
Of course if you provide a pattern requesting the display in meters, mainUI will show meters.
Regarding persistence, as UoM is not managed by persistence, my feeling is that it should store the value you provide in the updateState, so a value in kilimeters.

[lolodomo]

The first step is to check the unit of the value returned by the API. Any other discussion is probably just loss of time ;)

[dougculnane]

The API returns KM (totalMileage) and the value we want to display is in KM.

This is an example response I just got from the API:

{
    "data":{
        "type":"Car",
        "id":"VIN-ABC123",
        "attributes":{
            "fuelAutonomy":0.0,
            "fuelQuantity":0.0,
            "totalMileage":7653.79
        }
    }
}

The totalMileage is km. I use this value with the KM unit. But I guess... the value is converted to a standard meters value as a Number:Length type...???

[lolodomo]

The totalMileage is km. I use this value with the KM unit. But I guess... the value is converted to a standard meters value as a Number:Length type...???

There is no standard meters value.
Maybe a conversion specific to the mySQL persistence service?

[dougculnane]

@lolodomo OK there may be issues but they are outside the scope of this binding and pull request I guess.

Did you get a chance to look at the exception handling? Do you want to chat about this? We can do a quick meeting now if that would save time. I am not sure how i can resolve this.

[lolodomo]

Just check if KILO(METER( is the proper unit definition. I am almost sure other bindings are using kilometers.

I will finish the review probably tomorrow or Saturday,

[dougculnane]

Tesla uses MILEs...

openhab-addons/bundles/org.openhab.binding.tesla/src/main/java/org/openhab/binding/tesla/internal/TeslaChannelSelectorProxy.java

Lines 604 to 611 in 888cf00

	ODOMETER("odometer", "odometer", DecimalType.class, false) {
	@Override
	public State getState(String s, TeslaChannelSelectorProxy proxy, Map<String, String> properties) {
	State someState = super.getState(s);
	BigDecimal value = ((DecimalType) someState).toBigDecimal();
	return new QuantityType<>(value, ImperialUnits.MILE);
	}
	},

Volvo use KILO(METRE)

openhab-addons/bundles/org.openhab.binding.volvooncall/src/main/java/org/openhab/binding/volvooncall/internal/handler/VehicleHandler.java

Line 503 in 888cf00

return status.odometer != UNDEFINED ? new QuantityType<>((double) status.odometer / 1000, KILO(METRE))

[lolodomo]

So your code is correct IMHO.

Maybe there is a bug in MainUI.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/betatest-renault-ze-services-binding/72226/182

[lolodomo]

LGTM

[lolodomo]

I am concerned that a runtime error (like "SSL cert no long valid", or "Unknown host exception".... ) will throw a nasty error up the openHAB stack to be handled elsewhere.

You are using Jetty HTTP client in your code and you are handling the exceptions that can be thrown by Jetty calls. So I don't understand what exception you could loose.