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.

Sign up for GitHub

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

Jump to bottom

Basic arduino port of PULSOXYv0.0.1 #8

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Basic arduino port of PULSOXYv0.0.1 #8

wants to merge 11 commits into from

Conversation

seihan
Copy link

@seihan seihan commented Jan 18, 2020

This is an import of this repository: https://github.com/seihan/arduino-pulsoxy
with status from commit seihan/arduino-pulsoxy@cbdd3d9
into a subdirectory of the circuit boards repository.

Fixes: #7

$ git diff --stat cadus/master master
 Source_codes/SPO2/arduino-pulsoxy/ADC.c               |  39 ++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/ADC.h               |  16 ++++++
 Source_codes/SPO2/arduino-pulsoxy/AGC.c               | 143 +++++++++++++++++++++++++++++++++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/AGC.h               |  21 ++++++++
 Source_codes/SPO2/arduino-pulsoxy/LEDControl.c        | 121 ++++++++++++++++++++++++++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/LEDControl.h        |  19 +++++++
 Source_codes/SPO2/arduino-pulsoxy/README.md           |   9 ++++
 Source_codes/SPO2/arduino-pulsoxy/TC1_PWM_4kHz.c      |  26 +++++++++
 Source_codes/SPO2/arduino-pulsoxy/TC1_PWM_4kHz.h      |  16 ++++++
 Source_codes/SPO2/arduino-pulsoxy/TC2_8b_2ms.c        |  20 +++++++
 Source_codes/SPO2/arduino-pulsoxy/TC2_8b_2ms.h        |  14 +++++
 Source_codes/SPO2/arduino-pulsoxy/UART.c              |  41 +++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/UART.h              |  17 ++++++
 Source_codes/SPO2/arduino-pulsoxy/Update_Signals.c    |  44 ++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/Update_Signals.h    |  28 ++++++++++
 Source_codes/SPO2/arduino-pulsoxy/arduino-pulsoxy.ino | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/calculatingSPO2.c   |  55 +++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/calculatingSPO2.h   |  18 +++++++
 Source_codes/SPO2/arduino-pulsoxy/init_ADC.h          |  16 ++++++
 19 files changed, 821 insertions(+)

@seihan seihan mentioned this pull request Jan 18, 2020
Open
@CorCad
Copy link

CorCad commented Jan 31, 2020

@JosefKauer could you please review this PR? :)
Many thanks!

nopeslide
nopeslide requested changes Feb 20, 2020
View reviewed changes
Copy link

@nopeslide nopeslide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this port!
why did you copy all files into a new structure instead of using the existing files?
the minimal changes in the original UART.c & Update_Signals.h & main.c do not justify such a split.
This leads to either dead code, which won't be used in the future or at least duplicated code that gets hard to maintain. you could split up the original main.c into parts which can be used by by the old project and by your port.

Source_codes/SPO2/arduino-pulsoxy/UART.c Outdated Show resolved Hide resolved
@snue
Copy link

snue commented Mar 14, 2020

@nopeslide

why did you copy all files into a new structure instead of using the existing files?
the minimal changes in the original UART.c & Update_Signals.h & main.c do not justify such a split.

It's all in one directory because that's how Arduino projects work. The Arduino IDE does not look for files in subdirectories. This project is organized in a way that you can just open the .ino file in the Arduino IDE and hit compile/upload (with the right target board). No extra Makefiles, no Atmel Studio, plain Arduino IDE.

This leads to either dead code, which won't be used in the future or at least duplicated code that gets hard to maintain. you could split up the original main.c into parts which can be used by by the old project and by your port.

The way I understood issue #7 this should replace the Atmel Studio code. Not sure if there is another upstream repository for the PULSOXY software that you want to track for future improvements.

@nopeslide
Copy link

Thanks for the info, I didn't know this about the arduino toolchain.
Since all other solutions like symlinks etc would complicate the situation, I would say the copy is fine.

I had some issues reviewing this PR, because your initial copy already contained modifications (I couldn't easily determine your changes and ended up diffing each file by hand).
Could you rebase your commits on a clean copy of the individual files?

seihan and others added 11 commits March 14, 2020 19:25
@seihan @snue
arduino-pulsoxy: Import C sources from PULSOXYv0.0.1
6f6a300 
Copy .c and .h files from Driver/ Include/ Util/ and
main.c from the original project. Put everything in
one directory for the Arduino IDE to find everything.

Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Remove unused files
8a917c2 
CFile1.c and init_ADC.c do not contribute to the project.
Remove these files. Also remove init_ADC.h header file.
All ADC related functions are already declared and implemented
in the ADC.c/.h files.

Signed-off-by: Stefan Nuernberger <[email protected]>

Remove init_ADC.h header

Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Rename main.c to Arduino project file
95bbd08 
The .ino should have the same name as the containing
folder for the Arduino IDE to recognize it.

Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Fix F_CPU redefined warnings
63ec353 
F_CPU is usually already defined by the project makefile.

Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Include headers as extern "C"
ce10c3d 
Prevent C++ name mangling when reading these functions.

Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Fix typo
624de24 
Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Fix type mismatch in redeclaration
5f9f8b1 
These are ADC values that are used as signed int16 in the code.

Signed-off-by: Hannes Seidel <[email protected]>
@seihan @snue
arduino-pulsoxy: Fix missing newline in all files
c17c40f 
Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Add README file
7d27f53 
Signed-off-by: Stefan Nuernberger <[email protected]>
@seihan @snue
arduino-pulsoxy: Use setup() and loop() functions
36ef3f7 
Split the main() function into setup() and loop() for classical
Arduino feeling. Arduino will provide a main() that calls those.

Signed-off-by: Stefan Nuernberger <[email protected]>
@snue
arduino-pulsoxy: Fix trailing whitespace in all files
c90b6d6 
Signed-off-by: Stefan Nuernberger <[email protected]>
@snue
Copy link

snue commented Mar 15, 2020

Started from a clean working directory and force-pushed to update the PR. Also removed the unused init_ADC.h header and prefixed the commit messages with "arduino-pulsoxy" for the import in this repo.

$ git diff --stat cadus/master 
 Source_codes/SPO2/arduino-pulsoxy/ADC.c               |  39 ++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/ADC.h               |  16 ++++++
 Source_codes/SPO2/arduino-pulsoxy/AGC.c               | 143 +++++++++++++++++++++++++++++++++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/AGC.h               |  21 ++++++++
 Source_codes/SPO2/arduino-pulsoxy/LEDControl.c        | 121 ++++++++++++++++++++++++++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/LEDControl.h        |  19 +++++++
 Source_codes/SPO2/arduino-pulsoxy/README.md           |   9 ++++
 Source_codes/SPO2/arduino-pulsoxy/TC1_PWM_4kHz.c      |  26 +++++++++
 Source_codes/SPO2/arduino-pulsoxy/TC1_PWM_4kHz.h      |  16 ++++++
 Source_codes/SPO2/arduino-pulsoxy/TC2_8b_2ms.c        |  20 +++++++
 Source_codes/SPO2/arduino-pulsoxy/TC2_8b_2ms.h        |  14 +++++
 Source_codes/SPO2/arduino-pulsoxy/UART.c              |  44 ++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/UART.h              |  17 ++++++
 Source_codes/SPO2/arduino-pulsoxy/Update_Signals.c    |  44 ++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/Update_Signals.h    |  28 ++++++++++
 Source_codes/SPO2/arduino-pulsoxy/arduino-pulsoxy.ino | 161 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/calculatingSPO2.c   |  55 +++++++++++++++++++
 Source_codes/SPO2/arduino-pulsoxy/calculatingSPO2.h   |  18 +++++++
 18 files changed, 811 insertions(+)

@snue
Copy link

snue commented Mar 15, 2020

For reference, this code matches the status of the arduino-pulsoxy repo seihan/arduino-pulsoxy@a6a5245
Due to the import into a subdirectory with git format-patch | git am the line endings were automatically fixed, so that commit ended up empty and was dropped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snue snue snue left review comments

@nopeslide nopeslide nopeslide requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Microcontroller code needs rewrite
4 participants
@seihan @CorCad @snue @nopeslide