Skip to content

PPG Improvements Test 1 - #18

Open
znabF wants to merge 1 commit into
OpenSeizureDetector:mainfrom
znabF:ppg_improvements
Open

PPG Improvements Test 1#18
znabF wants to merge 1 commit into
OpenSeizureDetector:mainfrom
znabF:ppg_improvements

Conversation

@znabF

@znabF znabF commented Jun 30, 2026

Copy link
Copy Markdown

No description provided.

@jones139

Copy link
Copy Markdown
Member

Thank you for contributing this pull request - just a few 'administration' points.

  1. The change is named Ppg_test., but to build it in to the system it needs to have the same filename as the file it will replace (Ppg.) - please re-name them to be the original filename, then this pull request is asking me to replace the existing file with your new one.
  2. It would be good to have a description of what this pull request is doing. Something like "Replace the existing method of converting ppg reading to heart rate (which is xxxxx) with the method described by paper xxxxx which is to do xxxxxx.
  3. A comment in the code explaining what the new method is doing and the source reference would be good too (sorry if I missed it - I didn't see it from a quick scan).
  4. I have created a branch PPG_test - if you can work out how to propose your pull request against that, rather than main that would be good (but I can't remember how to do it - I always have to press random buttons!).
  5. I guess you have not compiled this to prove it runs yet? I'll update the devcontainer to get the build system working and let you know when I have done that so you can compile it yourself.

Thanks again for doing this!

Graham.

@jones139

Copy link
Copy Markdown
Member

I tried to update the PR to change Ppg.cpp rather than add Ppg_test.cpp, but I seem to have created my own branch instead, sorry (https://github.com/OpenSeizureDetector/PineTimeSD/tree/ppg_improvements).

The good news - it compiles without error :).

The bad news - it crashes on start-up on the PineTime device and the device re-boots.

I'll have a careful look at the code and see what is happening - it is probably an un-initialised variable or running off the end of an array.

@znabF

znabF commented Jul 27, 2026

Copy link
Copy Markdown
Author

Hey Mr. Jones, no worries at all! I'll take a look at the code and see what is causing it to crash. I apologize for the delay in my message.

If you were wondering about the paper I used as a reference for the code it was: https://ieeexplore.ieee.org/document/6091957 - Adaptive Comb Filtering for Motion Artifact Reduction from PPG
with a Structure of Adaptive Lattice IIR Notch Filter.

I also have other methods in mind we could try out but let me know if the start-up issue was resolved!

Thanks,
Zainab

@jones139

Copy link
Copy Markdown
Member

Hi, Sorry for the delay - I haven't had chance to get to this yet - my update to the OpenSeizureDatabase is taking more effort than usual because we have suddenly got a lot more data from a contributor, so I am having to update and test the processing pipeline.......

I think that to do this properly we really need to be able to see the raw ppg data, so you can test it offline and prove the algorithm works correctly, becuase we have no visibility of what it is doing once it is on the watch.

So we probably need to make a development build of PineTimeSD which will stream the raw ppg data over bluetooth (like we do accelerometer data), then I'll make a modified version of the OpenSeizureDetector phone app to receive it and log it alongside the accelerometer data (and hopefully a 'reference' heart rate monitor at the same time) - then we can save data while we do different activities and process it through different algorithms and pick one to implement.

I think that during InfiniTime development they might have had something that saved the raw ppg data - would you like to look into how to do this so that we can have some good data to work with? You might need to ask the InfiniTime folks or check the pull request or commit history on the InfiniTime project github to see if you can find it.

Sorry again that I haven't put any effort into this for you!

Graham.

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.

2 participants