greg/info-orbs
Watch
1
0
Fork
You've already forked info-orbs
 0
Code Issues 32 Pull requests 25 Projects Releases Packages Wiki Activity Actions

Features for ClockWidget and WeatherWidget #237

Open
nconner-06 wants to merge 12 commits from nconner-06/NC-dev into dev
pull from: nconner-06/NC-dev
merge into: greg:dev
Conversation 7 Commits 12 Files changed 13 +718 -149
nconner-06 commented 2025-02-19 20:08:22 +00:00 (Migrated from github.com)
Copy link

As posted on Discord, I've tried generating a pull request for the additional features I have developed

  • Changed the API used for TZ to not require an API key
  • Added support for multiple TZ - up to 5 in total
  • Added a new clock type MORPH
  • Added support for a Second Weather location
  • Added support for OpenWeatherMap API
  • Added support for Lat/Long locations
  • Added addition functionality to the web interface to support these new functionalities
  • Added minute support to the DIM function
  • Changed the Host name of the Orb to "ESP32-Orb"
  • And a few other minor changes.
As posted on Discord, I've tried generating a pull request for the additional features I have developed - Changed the API used for TZ to not require an API key - Added support for multiple TZ - up to 5 in total - Added a new clock type MORPH - Added support for a Second Weather location - Added support for OpenWeatherMap API - Added support for Lat/Long locations - Added addition functionality to the web interface to support these new functionalities - Added minute support to the DIM function - Changed the Host name of the Orb to "ESP32-Orb" - And a few other minor changes.
nconner-06 commented 2025-02-20 09:49:11 +00:00 (Migrated from github.com)
Copy link

Sorry, I just noticed there was a change to the config.h.template file.. I've uploaded a new copy to my branch.
Do I need to do anything to include it in this pull request or is it automatic?

Sorry, I just noticed there was a change to the config.h.template file.. I've uploaded a new copy to my branch. Do I need to do anything to include it in this pull request or is it automatic?
dreed47 commented 2025-03-02 08:02:44 +00:00 (Migrated from github.com)
Copy link

Your PR is greatly appreciated but you'll have a much better chance of getting these changes merged if you break this up into smaller PRs. Just a few suggestions. Try to keep one PR focused on only one feature or change. Also you have many many commits that don't have meaningful comments. It's best to have as few commits as possible and use comments that are meaning and relavant to tell code reviewers what, why, how you are changing this code. The other issue you'll have with too many changes in one PR is that your PR will quickly develop code conflict as it touches too many parts of the app. These code conflicts have to be resolved before you PR can be merged. Think about the PRs from the standpoint of the code reviewers. If the PRs are smaller and focused on only one change people are more likey to take the time to read the code and give you meaning feedback and help you get your changes into the code base. I'm interested in some of these changes myself particularly the OpenWeatherMap stuff

Your PR is greatly appreciated but you'll have a much better chance of getting these changes merged if you break this up into smaller PRs. Just a few suggestions. Try to keep one PR focused on only one feature or change. Also you have many many commits that don't have meaningful comments. It's best to have as few commits as possible and use comments that are meaning and relavant to tell code reviewers what, why, how you are changing this code. The other issue you'll have with too many changes in one PR is that your PR will quickly develop code conflict as it touches too many parts of the app. These code conflicts have to be resolved before you PR can be merged. Think about the PRs from the standpoint of the code reviewers. If the PRs are smaller and focused on only one change people are more likey to take the time to read the code and give you meaning feedback and help you get your changes into the code base. I'm interested in some of these changes myself particularly the OpenWeatherMap stuff
nconner-06 commented 2025-03-02 09:31:02 +00:00 (Migrated from github.com)
Copy link

Hi dreed47
Thanks for your feedback.. I must admit I got a little carried away developing changes and didn't stop to check point them into smaller sections.
If you want to 'leverage' any of my ideas, feel free... I'm more a 'problem solver' than a developer, so am good at providing 'snippets' of code which other people can combine to provide a fully working program.
But I'll try and split out a few of my changes into smaller PRs, like the TZ api change, etc all based on the DEV branch
Thanks again
Neil

Hi dreed47 Thanks for your feedback.. I must admit I got a little carried away developing changes and didn't stop to check point them into smaller sections. If you want to 'leverage' any of my ideas, feel free... I'm more a 'problem solver' than a developer, so am good at providing 'snippets' of code which other people can combine to provide a fully working program. But I'll try and split out a few of my changes into smaller PRs, like the TZ api change, etc all based on the DEV branch Thanks again Neil
dreed47 commented 2025-03-02 14:45:25 +00:00 (Migrated from github.com)
Copy link

Hi dreed47 Thanks for your feedback.. I must admit I got a little carried away developing changes and didn't stop to check point them into smaller sections. If you want to 'leverage' any of my ideas, feel free... I'm more a 'problem solver' than a developer, so am good at providing 'snippets' of code which other people can combine to provide a fully working program. But I'll try and split out a few of my changes into smaller PRs, like the TZ api change, etc all based on the DEV branch Thanks again Neil

I created a PR for setting the hostname https://github.com/brettdottech/info-orbs/pull/248

> Hi dreed47 Thanks for your feedback.. I must admit I got a little carried away developing changes and didn't stop to check point them into smaller sections. If you want to 'leverage' any of my ideas, feel free... I'm more a 'problem solver' than a developer, so am good at providing 'snippets' of code which other people can combine to provide a fully working program. But I'll try and split out a few of my changes into smaller PRs, like the TZ api change, etc all based on the DEV branch Thanks again Neil I created a PR for setting the hostname https://github.com/brettdottech/info-orbs/pull/248
nconner-06 commented 2025-03-03 15:29:31 +00:00 (Migrated from github.com)
Copy link

I'm starting to split these changes into smaller PR, first one is
Update to changeFormat function in ClockWidget to remove unneeded key press #256

I'm starting to split these changes into smaller PR, first one is Update to changeFormat function in ClockWidget to remove unneeded key press #256
flattermann commented 2025-03-04 01:32:26 +00:00 (Migrated from github.com)
Copy link

Let's keep this PR for reference, but merge the individual PRs.

Let's keep this PR for reference, but merge the individual PRs.
👍 1
nconner-06 commented 2025-03-06 15:45:45 +00:00 (Migrated from github.com)
Copy link

I just created a PR for the TZ api change in this package:

  • Added option for a second API provider for the timeZone information #269
I just created a PR for the TZ api change in this package: - Added option for a second API provider for the timeZone information #269
This pull request has changes conflicting with the target branch.
  • firmware/config/config.h.template
  • firmware/src/core/globaltime/GlobalTime.cpp
  • firmware/src/core/globaltime/GlobalTime.h
  • firmware/src/core/screenmanager/ScreenManager.h
  • firmware/src/core/utils/MainHelper.cpp
  • firmware/src/core/utils/MainHelper.h
  • firmware/src/widgets/clockwidget/ClockWidget.cpp
  • firmware/src/widgets/weatherwidget/WeatherWidget.cpp
  • firmware/src/widgets/weatherwidget/WeatherWidget.h
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin nconner-06/NC-dev:nconner-06/NC-dev
git switch nconner-06/NC-dev

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev
git merge --no-ff nconner-06/NC-dev
git switch nconner-06/NC-dev
git rebase dev
git switch dev
git merge --ff-only nconner-06/NC-dev
git switch nconner-06/NC-dev
git rebase dev
git switch dev
git merge --no-ff nconner-06/NC-dev
git switch dev
git merge --squash nconner-06/NC-dev
git switch dev
git merge --ff-only nconner-06/NC-dev
git switch dev
git merge nconner-06/NC-dev
git push origin dev
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something isn't working

  
conflicts

   
documentation
Improvements or additions to documentation

  
DO NOT MERGE
Work in progress not ready to be considered for merging

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
good first issue
Good for newcomers

  
hardware
Problems or ideas about the hardware

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
bug
conflicts
documentation
DO NOT MERGE
duplicate
enhancement
good first issue
hardware
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
greg/info-orbs!237
No description provided.