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

Refactoring waterelf32 #8

Open
cgmcintyr opened this issue Nov 15, 2017 · 10 comments
Open

Refactoring waterelf32 #8

cgmcintyr opened this issue Nov 15, 2017 · 10 comments

Comments

@cgmcintyr
Copy link
Collaborator

Issue for refactoring https://github.com/hamishcunningham/fishy-wifi/blob/c7e3cb415f0ee15786c1d1fe610efdc433e5f889/ardesp/waterelf32/waterelf32.ino

@layerzerolabs
Copy link
Collaborator

Less is more, this issue looks fine really, I'm just nit-picking; but I'd suggest that instead of 'issue for refactoring' saying just a few words about the actual issue e.g. Codebase has grown organically and would benefit from tidying up...

@Eroc33
Copy link
Collaborator

Eroc33 commented Nov 18, 2017

A suggestion I have that might fit into the vein of general refactoring is to turn on warnings (by default the arduino compiler suppresses all warnings, which imo is crazy) and fix them.

@hamishcunningham
Copy link
Owner

Good ideas.

Let's try and find some time, perhaps during the lecture slot today, to walk through the code a little? I vaguely remember several potential cleanups, but will need to study a little to bring them to mind...

Cheers, H

@EWLewis
Copy link
Collaborator

EWLewis commented Dec 2, 2017

Going to look at this issue this weekend.
See if i can make any changes by Tuesday (5th)

@cgmcintyr
Copy link
Collaborator Author

cgmcintyr commented Dec 2, 2017

I was thinking of moving from Arduino IDE to the esp IoT development framework. This would help with breaking the waterelf32 into modular components.

Have a read through the esp-idf docs and let me know what you guys think.

There's a project template we could use. Though we might want to create a new branch to limit disruptions to the current waterelf32 code.

@Eroc33
Copy link
Collaborator

Eroc33 commented Dec 2, 2017 via email

@cgmcintyr
Copy link
Collaborator Author

cgmcintyr commented Dec 2, 2017

@Eroc33 I was thinking more the flexibility of a Makefile, sdkconfig, and seperate directories for components.

For example there could be a separate directory for everything related to the webserver/webpage, for streaming data, for setting up sensors, for joinme, etc. See esp-idf's example project.

@Eroc33
Copy link
Collaborator

Eroc33 commented Dec 2, 2017

One big problem I see with that is that we probably lose the ability to use several of the sensor libraries we're depending on. Also afaict it's possible to have subdirectories in an arduino sketch. As far as makefiles go I'm not really sure of the advantages or disadvantages that would bring?

@EWLewis
Copy link
Collaborator

EWLewis commented Dec 2, 2017

I am going to take out the variable declarations that are above each function. Putting them into a header file, tidying up the code. Also look into putting the utility functions into a seperate file.

@layerzerolabs
Copy link
Collaborator

OK - good luck and look forward to seeing the pull request & documentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants