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

removed superfluous class name and Paamayim Nekudotayim from function calls #4

Merged
merged 2 commits into from
Feb 13, 2017

Conversation

kayno
Copy link
Contributor

@kayno kayno commented Feb 13, 2017

@SwiCago - thought I would wrap up just this change in one commit and pull request, as I figure it's better to have more small commits/merges than fewer larger ones!

RE: a public loop() function, I have been thinking about this and decided there is probably no need. I did some research and couldn't find anything that pointed to a standard practice of calling a library function "loop" if it should be called every iteration of the sketch's loop, so I think sync() is just fine. Plus it makes sense - it's keep the ESP8266 or whatever is running the library in "sync" with the heatpump. So I think it's good as is. Unless you see another reason?

I use http://home-assistant.io for controlling the heatpump and automation. I will upload my heatpump configuration file in case you are interested in trying it out. Will get you the link once I have it up on github.

@SwiCago
Copy link
Owner

SwiCago commented Feb 13, 2017

Agree, I have not seen many libraries use "loop", but would have been ok with an alias function, if you or others would have wanted it. We'll leave it. I chose sync, because that is exactly what it is doing.

@SwiCago SwiCago merged commit f6a5a96 into SwiCago:master Feb 13, 2017
@SwiCago
Copy link
Owner

SwiCago commented Feb 13, 2017

@kayno did widevane ever work on your heatpump. I can use remote to adjust it and the info packets return the reflected value, but via ESP8266 it never actually changes it. I know Hadley mentioned that somewhere in his Blog. Me personally don't care, but it would be nice if it worked like the remote.
I have one of those stupid Kumo CLoud devices(newer https version) I am hopeful that it doesn't actually confirm the cert and can maybe do man in the middle to see what it sends/receives from Kumo CLoud services.

@SwiCago
Copy link
Owner

SwiCago commented Feb 13, 2017

@kayno maybe we should see if the IR protocol data would work via the cn105 port. I don't see why it shouldn't. Here is a image describing the IR protocol, it is very similar to what we are sending, except it also supports I-See and wide vane settings.
https://github.com/r45635/HVAC-IR-Control/blob/master/Protocol/Mitsubishi_IR_Packet_Data_v1.1-FULL.pdf

@kayno
Copy link
Contributor Author

kayno commented Feb 13, 2017

@SwiCago I think I have just assumed WideVane worked, however to be honest I can't recall actually testing it! I will try tonight when I get home, as well as the vertical vane. I know power/mode/fan settings work as we use these all the time, but we just leave the vanes on SWING. I will test everthing tonight.

The document you found for the IR protocol is great, and looks similar, if not the same to what we are doing. Looks like we need to break down some bytes into bits as well to get exact settings, but that won't be impossible. I don't think my model has i-see, but it has i-save, powerful, and econo-cool, so perhaps a bit level breakdown will reveal all of those settings.

Will get back to you with my testing.

@jarrod180
Copy link

It looks like the json data is expected to contain the key "windVane", I can't see a reason for this shouldn't it be "wideVane"?

@SwiCago
Copy link
Owner

SwiCago commented Feb 14, 2017

@kayno all the other settings work on my unit, just not the wide vane. As mentioned, if i change it with the remote, i get it back via the sync of the esp...but if i set it with esp it is ignored. I am sure we'll figure it out. And yes, the IR look very close. Maybe we should add a method that allows us to send custom bytes, minus the lead 0xfc and trailing chksum, let the code add those but pass through all other bytes w/o verification. What do you think?

@SwiCago
Copy link
Owner

SwiCago commented Feb 14, 2017

@jarrod180 fixed the typo...kayno, jarrod pointed out a typo in your mqtt sample. I went ahead and fixed it

@kayno
Copy link
Contributor Author

kayno commented Feb 14, 2017

@jarrod180 excellent pick up, thanks! I have checked over the rest of the code for that typo, but it seems I only made it once. I will update my fork to get @SwiCago's change.

@SwiCago i think a public method to send custom bytes might be good for testing, but probably something that should only be used for that! The heatpump's can get annoyed if you send it bad data, as I found out. I thought I had fried mine as it went into "broken" mode (flashing operation light) and the manual said "call a service technician"! Thankfully a power down at the mains isolating switch for more than 60 seconds resolved this.

@kayno
Copy link
Contributor Author

kayno commented Feb 14, 2017

@SwiCago @jarrod180 i'm thinking that github for these discussions is working much better than hadley's blog or esp8266 forum where constant moderation is required.

perhaps though to avoid cluttering these pull request comments with other discussions, we use issues? e.g. separate issues for each bug or feature/enhancement request etc. An issue for adding a public method to send custom packets to the heatpump (we can discuss/decide if thats a good idea there) for example. The wiki could be used for sharing info on using the library and also for describing how it can be integrated with home assistant (HASS), openHAB, etc.

Just a thought! I'm thinking this collaboration is working well and want to make sure we use the best tools in the right way to make it easier :)

@SwiCago
Copy link
Owner

SwiCago commented Feb 14, 2017

@kayno fixed the simple demo sketch to work with latest library. Added the custom send packets method.

@SwiCago
Copy link
Owner

SwiCago commented Feb 14, 2017

@kayno, just saw your comments. Good idea about the use of issues. Honestly I am fairly new to the online git thing. Mostly only to git via terminal

@jarrod180
Copy link

jarrod180 commented Feb 14, 2017

@kayno , Yes, same as SwiCago, only just signed up here and getting my ESP's set up for my heat pumps.
Seems like a good idea to use the features of the site properly.

@lekobob lekobob mentioned this pull request Mar 4, 2017
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

Successfully merging this pull request may close these issues.

None yet

3 participants