search

homieiot / homie-esp8266

💡 ESP8266 framework for Homie, a lightweight MQTT convention for the IoT
http://homieiot.github.io/homie-esp8266
MIT License
1.36k stars 307 forks source link

Fix Config.cpp cast warnings #734

Closed Keiishu closed 2 years ago

Keiishu commented 2 years ago

There were two warnings caused by implicit HomieBootMode to int casts in Config.cpp.

Warnings ``` (...)\Config.cpp:191:28: warning: format '%d' expects argument of type 'int', but argument 3 has type 'HomieBootMode' [-Wformat=] 191 | bootModeFile.printf("#%d", bootMode); (...)\Config.cpp:193:69: warning: format '%d' expects argument of type 'int', but argument 3 has type 'HomieBootMode' [-Wformat=] 193 | Interface::get().getLogger().printf("Setting next boot mode to %d\n", bootMode); ```

Explicitly casting to int solved the issue.

Keiishu commented 2 years ago

I believe the build failed because of #731. I see it has been fixed here on this commit https://github.com/labodj/homie-esp8266/commit/e4bcb83fa916a9cbc1d9148d2e56435ec4b4caad. Maybe we could give it a try?

luebbe commented 2 years ago

I believe the build failed because of #731. I see it has been fixed here on this commit labodj@e4bcb83. Maybe we could give it a try?

How did you find this fix? I don't see a PR for this. So we have to copy&paste it from @labodj? Could you @Keiishu please try and test it, and, if possible make a PR? I doubt I'll be able to look into it before next week.

labodj commented 2 years ago

Hi @luebbe I can make a PR if you want, the code in my repo works with the latest platform/libraries

For that fix in particular I looked for changes in esp32 platform library

luebbe commented 2 years ago

yes please!

Keiishu commented 2 years ago

I believe the build failed because of #731. I see it has been fixed here on this commit labodj@e4bcb83. Maybe we could give it a try?

How did you find this fix? I don't see a PR for this. So we have to copy&paste it from @labodj? Could you @Keiishu please try and test it, and, if possible make a PR? I doubt I'll be able to look into it before next week.

I was looking at the current forks and stumbled across @labodj's, which had recent interesting commits, including a fix for the ESP32.

I see the PR #735 has already been created so I'll leave that to you.