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

[swagger-codegen] IllegalArgumentException TadoModeTerminationCondition: declares multiple JSON fields named 'type' #16779

Closed
dandjo opened this issue May 20, 2024 · 16 comments · Fixed by #16836
Assignees
Labels
bug An unexpected problem or unintended behavior of an add-on

Comments

@dandjo
Copy link

dandjo commented May 20, 2024

Current Behavior

An error occurs when initializing or interacting with the tado addon in version 4.2.0 M3.

2024-05-20 16:16:58.863 [WARN ] [mmon.WrappedScheduledExecutorService] - Scheduled runnable ended with an exception: 
java.lang.IllegalArgumentException: Class org.openhab.binding.tado.internal.api.model.TadoModeTerminationCondition declares multiple JSON fields named 'type'; conflict is caused by fields org.openhab.binding.tado.internal.api.model.TadoMo
deTerminationCondition#type and org.openhab.binding.tado.internal.api.model.OverlayTerminationCondition#type
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:302) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:130) ~[?:?]
        at com.google.gson.Gson.getDelegateAdapter(Gson.java:652) ~[?:?]
        at org.openhab.binding.tado.internal.api.RuntimeTypeAdapterFactory.create(RuntimeTypeAdapterFactory.java:60) ~[?:?]
        at com.google.gson.Gson.getAdapter(Gson.java:556) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.createBoundField(ReflectiveTypeAdapterFactory.java:160) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:294) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:130) ~[?:?]
        at com.google.gson.Gson.getAdapter(Gson.java:556) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.createBoundField(ReflectiveTypeAdapterFactory.java:160) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:294) ~[?:?]
        at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:130) ~[?:?]
        at com.google.gson.Gson.getAdapter(Gson.java:556) ~[?:?]
        at com.google.gson.Gson.fromJson(Gson.java:1226) ~[?:?]
        at com.google.gson.Gson.fromJson(Gson.java:1137) ~[?:?]
        at com.google.gson.Gson.fromJson(Gson.java:1047) ~[?:?]
        at com.google.gson.Gson.fromJson(Gson.java:1014) ~[?:?]
        at org.openhab.binding.tado.internal.api.client.HomeApi.showZoneState(HomeApi.java:529) ~[?:?]
        at org.openhab.binding.tado.internal.handler.TadoZoneHandler.getZoneState(TadoZoneHandler.java:129) ~[?:?]
        at org.openhab.binding.tado.internal.handler.TadoZoneHandler.updateZoneState(TadoZoneHandler.java:325) ~[?:?]
        at org.openhab.binding.tado.internal.handler.TadoZoneHandler$1.run(TadoZoneHandler.java:420) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) ~[?:?]
        at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305) ~[?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
        at java.lang.Thread.run(Thread.java:840) [?:?]

Environment

  • Version used: openHAB 4.2.0-M3, Add-on 4.2.0-M3
  • Operating System and version: Ubuntu server, 5.15.0-1055-raspi, openhabian
@dandjo dandjo added the bug An unexpected problem or unintended behavior of an add-on label May 20, 2024
@openhab-bot
Copy link
Collaborator

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

https://community.openhab.org/t/illegalargumentexception-tadomodeterminationcondition-declares-multiple-json-fields-named-type/156112/2

@dandjo
Copy link
Author

dandjo commented May 20, 2024

Here are additional information from my things and items used:

Tado Home Thing

UID: tado:home:daniel
label: Tado Home
thingTypeUID: tado:home
configuration:
  password: xxxXXXxxxXXX
  username: john.doe@email.not
channels:
  - id: homePresence
    channelTypeUID: tado:homePresence
    label: At Home
    description: ON if at home, OFF if away
    configuration: {}

Tado Air Conditioning Thing

UID: tado:zone:daniel:2
label: Tado Air Conditioning
thingTypeUID: tado:zone
configuration:
  hvacChangeDebounce: 5
  fallbackTimerDuration: 30
  id: 2
  refreshInterval: 30
bridgeUID: tado:home:daniel
channels:
  - id: currentTemperature
    channelTypeUID: tado:currentTemperature
    label: Temperature
    description: Current temperature
    configuration: {}
  - id: hvacMode
    channelTypeUID: tado:hvacMode
    label: HVAC Mode
    description: Mode of the device (OFF, HEAT, COOL, DRY, FAN, AUTO - if supported)
    configuration: {}
  - id: targetTemperature
    channelTypeUID: tado:targetTemperature
    label: Target Temperature
    description: Thermostat temperature setpoint
    configuration: {}
  - id: fanspeed
    channelTypeUID: tado:fanspeed
    label: Fan Speed
    description: AC fan speed (only if supported by AC)
    configuration: {}
  - id: swing
    channelTypeUID: tado:swing
    label: Swing
    description: State of AC swing (only if supported by AC)
    configuration: {}
  - id: overlayExpiry
    channelTypeUID: tado:overlayExpiry
    label: Overlay End Time
    description: Time until when the overlay is active. Null if no overlay is set or
      overlay type is manual.
    configuration: {}
  - id: timerDuration
    channelTypeUID: tado:timerDuration
    label: Timer Duration
    description: Total duration of a timer
    configuration: {}
  - id: operationMode
    channelTypeUID: tado:operationMode
    label: Zone Operation Mode
    description: Active operation mode (schedule, manual, timer or until next change)
    configuration: {}
  - id: acPower
    channelTypeUID: tado:acPower
    label: AirCon Power State
    description: Indicates if the air-conditioning is Off or On
    configuration: {}
  - id: openWindowDetected
    channelTypeUID: tado:openWindowDetected
    label: Open Window Detected
    description: Indicates if an open window has been detected
    configuration: {}
  - id: humidity
    channelTypeUID: system:atmospheric-humidity
    label: Humidity
    description: Current humidity in %
    configuration: {}

Tado Aircon Power State Item (example for all other)

label: Tado AirCon Power State
type: Switch
category: switch
groupNames:
  - tado
tags:
  - Point

@andrewfg andrewfg self-assigned this May 22, 2024
@lolodomo
Copy link
Contributor

Was it working in milestone 2 ?
As the only change regarding Tado in milestone 3 was reverted by #16793 , can you please tell us if your problem persists in the current daily snapshot ?

@andrewfg
Copy link
Contributor

Was it working in milestone 2 ?

@lolodomo I am still on M2 and it is working just fine.

@lolodomo
Copy link
Contributor

So as the only change in M3 was reverted, we can expect the issue to be now resolved ?
Can you check again with the last snapshot ?

@andrewfg
Copy link
Contributor

Can you check again with the last snapshot ?

Ok. Probably tomorrow..

@dandjo
Copy link
Author

dandjo commented May 25, 2024

Confirming, with snapshot 4.2.0~S4095 the error is gone, so the issue seems to be resolved with #16793.

@dandjo
Copy link
Author

dandjo commented May 25, 2024

For future versions it would probably be a better strategy to adapt the Tado addon to the newer Swagger version.

@lolodomo
Copy link
Contributor

Closing as issue is no more present.

@andrewfg
Copy link
Contributor

andrewfg commented May 26, 2024

it would probably be a better strategy to adapt the Tado addon to the newer Swagger version

@lolodomo / @dandjo even though this issue has been closed I am keeping the above suggestion in my mind. The problem is that it is not so easy to fix. The problem is that the Tado Cloud API does actually use JSON payloads with fields named 'type' and it seems the latest version of Swagger correctly parses the Tado YAML and generates DTOs for the API payloads with API related fields named 'type' annotated with '@SerializedName("type") but it apparently generates extra fields with the same '@SerializedName("type") annotation. I am not sure why that would happen with the new Swagger version. There may hopefully be a compatibility switch to turn off that new behaviour (i.e. make the new Swagger version behave like the older version), but I would need to study Swagger in my spare time. In the meantime the way to make Swagger behave like the older version is to -- well -- use the older version..

@andrewfg
Copy link
Contributor

@andrewfg
Copy link
Contributor

andrewfg commented May 31, 2024

I am reopening this issue because I think I have identified the cause of the problem, yet personally lack the skills to solve it, where there are other OH users (e.g. @dfrommi ) who may be able to fix it.

The issue is described here. In short the Swagger Codegen (used to build the Tado binding) v2.3.1 does NOT generate duplicate fields in child DTOs whereas v2.4.41 DOES generate such duplicate fields. From deep googling I think there is some Swagger Codegen setting property called 'supportsInheritance` that may influence the creation of such duplicate fields in child classes.

However I lack the Swagger skills to know a) if the above hypothesis is correct, or b) how to fix it.

EDIT: perhaps more info here

@andrewfg andrewfg reopened this May 31, 2024
@andrewfg andrewfg changed the title [tado] IllegalArgumentException TadoModeTerminationCondition: declares multiple JSON fields named 'type' [swagger-codegen] IllegalArgumentException TadoModeTerminationCondition: declares multiple JSON fields named 'type' May 31, 2024
@dfrommi
Copy link

dfrommi commented May 31, 2024

There's always the option to stop using the generator.
Just copy the generated code that works and commit it, then remove the generator-step. From then on, maintain the DTOs manually.

I've initially created the generator to simplify API integration in OH in general and without introducing extra dependencies. But as far as I know, it's still only used in the tado integration and never found a wider acceptance.

So I would suggest, get rid of the unicorn.
As I'm not using OH anymore for a long time now, I will also not spend time keeping the generator up to date. So this step makes most likely a lot of sense, regardless of the issue.

Hopefully my point of view helped at least a little bit.

@andrewfg
Copy link
Contributor

andrewfg commented May 31, 2024

get rid of the unicorn.

@holgerfriedrich @lolodomo => WDYT? The YAML has not changed since years, so the CodeGen produced DTOs will have not changed either (even though Swagger CodeGen recreates them on every CI build). So I can easily do the copy paste of the existing files from /target/generated-code/src/main/java/... to src/main/java/.. .. Personally I can't see any down sides, but WDYT?


EDIT: it is not as simple as just copy/paste (I just tried that and failed). The auto-generated sources were excluded from spotless, so when copied over they produce many tens of build warnings, such as missing Copyright header, missing Java doc author tag, missing Null annotations, and many potential NPE warnings. However this is all fairly trivial stuff, so I could do it in a day or two..

@holgerfriedrich
Copy link
Member

@andrewfg I do not have a strong opinion here. I don't know the internals of the add-on.

Right now we are stuck with a 2018 version of swagger-codegen-maven-plugin, but is there a reason to change it?
I am not sure if it it worth the efforts. If the tado add-on is still actively used and developed, then maybe it is.

@andrewfg
Copy link
Contributor

@holgerfriedrich I suppose the "only" risk is if the Swagger Codegen v2.3.1 is no longer downloadable and we know that later versions don't work. I am inclined to static copy the java files into OH so that we have everything in house.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants