ecocurious2 / MultiGeiger

Geigerzähler mit ESP32 und empfindlichem Si22g-Zählrohr (Gamma)
https://multigeiger.readthedocs.io/
GNU General Public License v3.0
71 stars 30 forks source link

Local alarm compile #427

Closed t-pi closed 3 years ago

t-pi commented 3 years ago
t-pi commented 3 years ago

Adresses #416

ThomasWaldmann commented 3 years ago

can you please update your master branch and rebase this onto the current master?

t-pi commented 3 years ago

this is mixing a fix/optimization(?) to existing hard-to-get-right code (the sequencer isr / mutex) with a new feature in the same commit. please avoid.

Question: How can I work on basically two branches at the same time? E.g. for the alarm I needed to work on speaker, that's why this got mixed up. Is there a preferred way to work on "lower level" commits in one branch and "higher level" commits in a second one?

t-pi commented 3 years ago

Fixes #416

ThomasWaldmann commented 3 years ago

You could do 2 branches and (re)base the feature branch onto the bugfix branch now and then.

master -> bugfix -> feature

Of course, there could be conflicts now and then depending on how changes overlap.

Or just do the same within 1 branch, but keep fixing and adding stuff in separate commits always. If you need multiple commits for each, you can later use git rebase -i and s(quash) or f(ixup) to merge some commits again, e.g squash fix1 fix2 fix3 commits, squash feature1 feature2 feature2 commits, so you'll end up with 1 fix commit and 1 feature commit.

ThomasWaldmann commented 3 years ago

I am still having troubles to review this / to see why the ISR code got changes / whether they improve the code.

So, may I suggest:

ThomasWaldmann commented 3 years ago

Thanks!