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

Remove unnecessary JSON nodes #14

Open
cterwilliger opened this issue Sep 26, 2020 · 6 comments
Open

Remove unnecessary JSON nodes #14

cterwilliger opened this issue Sep 26, 2020 · 6 comments

Comments

@cterwilliger
Copy link
Contributor

Remove JSON nodes required by early version of node-red and configure their upstream nodes to provide JSON output. Do we expect this will improve performance or only simplify the flow (which is still a bonus). I'll volunteer to do it...

@greendog99
Copy link
Collaborator

I think it just simplifies the flow.

@greendog99
Copy link
Collaborator

greendog99 commented Sep 26, 2020

Thinking about this a bit more, and since this change adds no tangible benefit, I wonder if it makes more sense to start from scratch on the flows to see what other optimizations can be made... my new Canbus decoder provides an easier way to get at some of the RVC data, which could also simplify things.

@cterwilliger
Copy link
Contributor Author

Sure, that makes sense. I looked at your new code, it does look simpler and cleaner than the old perl code, but now I have to come up to speed on Ruby... So, are you running a separate RPi with the new code to build and test? Wondering if I can start substituting Ruby for Perl modules on the same RPI.

@greendog99
Copy link
Collaborator

I am running a separate RPi for development efforts, but you could easily run both on the same RPi. I think Python would be ideal as it's more widely used than Ruby, but I'm not nearly as proficient in it so I went with Ruby. Feel free to throw out other ideas... I just really wanted to ditch Perl :-)

@cterwilliger
Copy link
Contributor Author

I've got several changes & enhancements, but don't know if you want to throw those in to legacy while working on the Ruby platform. 1. I needed to merge my batteries on command, so I created that. 2. I don't like Temps displayed in tenths, there's no way the sensors are that good anyway, so I removed that. 3. I implemented Pushover where you left a blank space for it. 4. Added inverter temperatures to monitoring after one of my neighbors experienced bad fans. 5. added battery bay temp

@greendog99
Copy link
Collaborator

I also set up a battery merge for my personal use.

I think 1, 4, and 5 are probably specific to certain coach models and years. Do you have it set up so those only show up in the UI by request or by year/model? If not, I could help get that part done since that code can be tricky. Knowing what features exist on each model and year has been one of the hardest things to figure out though. For 2, I like the tenths so I can at least see if/when the thermostat might be ready to turn on/off and if the temp reading is actually changing (e.g. on really hot days when the AC is having a hard time keeping up). We could make it a configurable option. For 3, we could include Pushover for everyone along with a little text explaining what it is. I originally took it out since I figured it wouldn't interest 99% of users and had the potential to confuse people.

As for whether or not to include these changes in legacy or not, my preference is to try to forge forward with something new (and hopefully easier to modify / maintain) unless some of these would have a big impact/win for existing users.

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

2 participants