merging unreality and kayno forks #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g, clean up a little
- started magic number clean up, - synced wantedSettings with currentSettings on connect(), - renamed 'direction/DIR' to 'wideVane' for the horizontal vane (matching terminology used in manual/on the remote) - re-worked the naming of the MAP lookup functions so that a) you call the same function name regardless of string vs int (overloaded parameters) and b) tried to make the names more descriptive
- fixed typo in hp.checkForUpdate() call - changed hp.getSettings() to get settings back rather than pass them in - updated settings options to match options in code
|
Reviewed ...Looks good and from what I see, we were not far apart. I initially wanted to use structs, but didn't understand yet how to construct them LOL. As said in a chat, this is my first C library in about 20yrs, so fresh eyes on it is a great benefit. Merging |
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi SwiCago
I have merged your commits that I missed (because I forked from unreality) into my fork, and blended my changes as well as some of unreality's changes into your code. This pull request shows the result of that!
From my fork:
From unreality's fork:
I have tested this successfully on my heatpump, with the MQTT example, and it's working well - I haven't had any issues with it. I still have some more plans/ideas, but I want to at least get this merge done first to clean up how I forked from unreality and missed your commits.
Happy to discuss everything and anything before any merging happens.
Cheers :)
kayno