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

Code refactor and cleanup. #8

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

leonxx-dev
Copy link

Hi, I really enjoy your project. This was my first time working with Arduino. I did not made any changes to the functionality, I just organize the code add some comments so people who is just started with IoT can better understanding about the parts of your project.

I moved all the sensitive data to separate file called "env.h" the example of this file is "env_example.h". This made for users who working with version control system like a Git, so u don't need to clean up passwords and API keys all the time before commiting the code.

Example if new person will clone this repo he need to create env.h locally and copy all from env_example.h with his own data. And he can commit without fear of commiting sensitive information.

I change NTP Client library for the native time.h library. Because NTP was missing date.

Hopefully it will be helpful.

@soharddbwarez
Copy link

Hi @Leon2ev thanks for sharing your code, even though it isn't merged I've been able to figure it out and have your code working, I do find your code better, it's more refined and the comments are helpful to see what the code is exactly doing.

@VolosR you might want to merge this code with yours, I think it improves your original code and it's working perfectly.

@leonxx-dev
Copy link
Author

@soharddbwarez thank you for your feedback it's very important to me! I think @VolosR is a great guy and he creates a cool projects, but he don't want to colaborate.

@VolosR
Copy link
Owner

VolosR commented Dec 15, 2022

Thank you for fixing and improving my code. I am sorry you think i dont want to colaborate. I want but belive me i am quite bussy. i am not profesional youtuber. I have more and more trouble to find free min to make videos i own to my sponsors. I hope you understand. I will share your code and give you credits . Thank you again.

@soharddbwarez
Copy link

@Leon2ev well, it's his loss when he doesn't collaborate. Maybe it helps that I'm using your code and that I can confirm that it is working and an improvement he reconsiders to merge the project.

@soharddbwarez
Copy link

Hi @Leon2ev I found a bug in using your code, it does work but whenever I disconnect the power and need to do a restart or whenever I start it first time I see the first screen to show it's getting connected to the network which goes fast, then on the main screen of the WeatherStation I'm getting 3 different situations:

  1. Everything will get loaded fast and there's no issue, this is the best outcome of course because it works normal.
  2. It loads the time, town and the seconds fast and displays it, the animation however loads but the animation is so slow that I see the next frame every 3 seconds or so, also the temperature and humidity has an issue and doesn't get any data so I'm seeing null on the display, this doesn't get loaded and I've waited for as long as I'm writing this post which is about ten minutes now so I'm pretty sure it will not load the data at all, the animation will start playing normal after a few seconds.
  3. This situation it's the other way around with the data being displayed so the temperature and humidity loads fast, the animation keeps playing very slow and doesn't recover and now the time, date and seconds don't load at all.

So what happens most of the time is that I get either the second or third situation and sometimes it loads everything normally which isn't good enough because it's unreliable and I want to get that improved but at this point I have no idea where to look.

I'm using Git so I'm keeping history updated and I've used your code for the main part so if you want to take a look at the project as I'm using it you can go to my GitHub page here: https://github.com/soharddbwarez/MyArduinoProjects/tree/master/WeatherStation

@leonxx-dev
Copy link
Author

@soharddbwarez Hello my friend. Is this bug happens only after restart, power loss? If you turn it on first time it will work as it should? I have ESP32 at my office and I'll be there tomorrow, so I can take a look.

@soharddbwarez
Copy link

@leonxx-dev Happy New Year, the bug happens every time when I turn it on, then I have to use reset button until it finally loads all the resources at once, it seems to only work correctly if everything is loaded, pulled from the website and it can display it all at once, whenever there's any part that doesn't load instantly during start I can forget it getting loaded later on so I have to reset until it works.

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.

3 participants