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

Recent changes break bi-direction communication on MSZ-GL06NA #207

Closed
stewartadam opened this issue Nov 18, 2023 · 9 comments
Closed

Recent changes break bi-direction communication on MSZ-GL06NA #207

stewartadam opened this issue Nov 18, 2023 · 9 comments

Comments

@stewartadam
Copy link

I'm using this library via esphome-mitsubishiheatpump.

Commit cea90c5 works as expected, but any commit later causes bi-directional serial communication to silently fail. Updates sent do nothing and no updates are received from the heat pump, so the current setpoint shows as NaN.

@SwiCago
Copy link
Owner

SwiCago commented Nov 29, 2023

@prashker , this relates to the commit you made, can you review pls

@prashker
Copy link
Contributor

@SwiCago if I'm reading the above correctly, my commit works - it is subsequent commits breaking communication.

@stewartadam seems to indicate that problem is immediately after my commit, which would be ce460c3 by @dzungpv

@dzungpv
Copy link
Contributor

dzungpv commented Nov 30, 2023

@SwiCago if I'm reading the above correctly, my commit works - it is subsequent commits breaking communication.

@stewartadam seems to indicate that problem is immediately after my commit, which would be ce460c3 by @dzungpv

I am running for months without any issue. Both with esp8266 and esp32. With my changes, if you are not set the custom UART pin on ESP32, it use default settings. @stewartadam can you try my project, it is not ESPHOME but you can try to test if the bug happen with this lib or ESPHOME implementation

@markalston
Copy link

@dzungpv It looks like your issue is that you are only setting the values for rxPin and txPin inside the

if (rx >= 0 && tx >= 0) {

test.

If on the initial call to connect, rx and tx are set to -1 then rxPin and txPin are not set but you use them on line 119.

_HardSerial->begin(bitrate, SERIAL_8E1, rxPin, txPin);

Maybe you could initialize them to -1 and add a test to check value before line 119 in case it is -1 and then fallback to the old

_HardSerial->begin(bitrate, SERIAL_8E1);

This is my guess to the issue anyway.

@dzungpv
Copy link
Contributor

dzungpv commented Dec 1, 2023

@dzungpv It looks like your issue is that you are only setting the values for rxPin and txPin inside the

if (rx >= 0 && tx >= 0) {

test.

If on the initial call to connect, rx and tx are set to -1 then rxPin and txPin are not set but you use them on line 119.

_HardSerial->begin(bitrate, SERIAL_8E1, rxPin, txPin);

Maybe you could initialize them to -1 and add a test to check value before line 119 in case it is -1 and then fallback to the old

_HardSerial->begin(bitrate, SERIAL_8E1);

This is my guess to the issue anyway.

It is original code, I just store the custom RX TX pin for call in retry you can check change here: dzungpv@ce460c3 and 4 of my units running fine since, 3 esp8266 and 1 esp32
Update: because I am using custom pin so ESP32 not show error @SwiCago you can check the PR to fix the bug

@dzungpv dzungpv mentioned this issue Dec 1, 2023
@dzungpv
Copy link
Contributor

dzungpv commented Dec 1, 2023

@stewartadam Can you check latest change?

@stewartadam
Copy link
Author

Just tried it, confirmed working! Thank you @dzungpv and @markalston

SwiCago added a commit that referenced this issue Dec 4, 2023
@SwiCago
Copy link
Owner

SwiCago commented Dec 4, 2023

Thanks @dzungpv , @stewartadam , @markalston
Great work of finding a solution for everyone fast...Pull has been merged.

@SwiCago SwiCago closed this as completed Dec 4, 2023
@prashker
Copy link
Contributor

prashker commented Dec 5, 2023

Teamwork!

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

No branches or pull requests

5 participants