mcci-catena / Catena-Arduino-Platform

Arduino platform library for MCCI Catena IoT Systems
MIT License
12 stars 11 forks source link

update "Fix #253: Use AdcStart() before each ReadAnalog value" #256

Closed dhineshkumarmcci closed 4 years ago

dhineshkumarmcci commented 4 years ago

Advanced the version number in README.md and CatenaBase.h

dhineshkumarmcci commented 4 years ago

@terrillmoore changes in the file CatenaStm32L0_ReadAnalog.cpp of the branch issue253 is not reflected in the master during the merge of this pull-request. https://github.com/mcci-catena/Catena-Arduino-Platform/blob/88a771a198371a233de3b6867df86c9fd5d03ba1/src/lib/stm32/stm32l0/CatenaStm32L0_ReadAnalog.cpp#L174-L177 Trying to create a new pull-request with branch issue253 says no changes to compare. Is that ok to create a new branch and submit a pull-request?

terrillmoore commented 4 years ago

It would be best to understand how this happened. I suggest that you fork this repo, first apply your pull request there, then you can push changes upstream from your repo once you are sure they are correct.

terrillmoore commented 4 years ago

I think the merge didn't pick up your changes -- you committed changes to issue253 after I merged. Reopen #253, rename your local branch to "issue253a" or something, and submit a new pr.

terrillmoore commented 4 years ago

Note this from the history above: image The highlighted commits appear after the [Merged] indication. As far as I know, there is no way to merge the extra commits without a new PR. They don't point to the reference lines.

More research points to #254. I reverted that, because the documentation was not done.

Proper process was not followed. Proper process is to push additional changes to the branch used for PR #254, not to create a new branch. I am going to REVERT this PR as well, and you must push ALL your changes onto either #254, or a different new PR. Please get review from @mcciyssaroha for this, thanks.