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

Generic history for different sensors #263

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Generic history for different sensors #263

wants to merge 9 commits into from

Conversation

Kampi
Copy link
Collaborator

@Kampi Kampi commented Apr 22, 2024

First idea and not tested.

@Kampi Kampi added the enhancement New feature or request label Apr 22, 2024
@Kampi Kampi requested a review from jakkra April 22, 2024 13:26
jakkra
jakkra approved these changes May 3, 2024
View reviewed changes
Copy link
Owner

@jakkra jakkra left a comment

Choose a reason for hiding this comment

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

Looks awesome!

app/src/history/zsw_history.c Outdated Show resolved Hide resolved
@Kampi
Copy link
Collaborator Author

Kampi commented May 3, 2024

Looks awesome!

But doesn´t work yet :D

@Kampi Kampi force-pushed the Add-Generic-History branch 3 times, most recently from 91dafb9 to 4c58bc2 Compare May 7, 2024 11:30
jakkra
jakkra requested changes May 22, 2024
View reviewed changes
app/src/history/zsw_history.c Outdated Show resolved Hide resolved
app/src/history/zsw_history.c Show resolved Hide resolved
app/src/history/zsw_history.c Outdated Show resolved Hide resolved
app/src/history/zsw_history.c Show resolved Hide resolved
app/src/history/zsw_history.c Show resolved Hide resolved
@jakkra
Copy link
Owner

jakkra commented May 22, 2024

I think we should include num_stored_samples in the zsw_history_t to allow the user to know how many of the samples are actually valid. Since the num param today is always the max number of samples. Or can num be changed to actually reflect the number of valid samples?

@jakkra
Copy link
Owner

jakkra commented May 22, 2024

I think we should include num_stored_samples in the zsw_history_t to allow the user to know how many of the samples are actually valid. Since the num param today is always the max number of samples. Or can num be changed to actually reflect the number of valid samples?

Because in the battery app now it plots all samples, which are all 0 so it's hard to see anything.

@Kampi
Copy link
Collaborator Author

Kampi commented May 22, 2024 •
edited

I think we should include num_stored_samples in the zsw_history_t to allow the user to know how many of the samples are actually valid. Since the num param today is always the max number of samples. Or can num be changed to actually reflect the number of valid samples?

Because in the battery app now it plots all samples, which are all 0 so it's hard to see anything.

I think we should keep num (but rename it to length) to have an indicator about the maximum length. Then we add a sample counter. Or we can use write_index for it?

@jakkra
Copy link
Owner

jakkra commented May 22, 2024

I think we should include num_stored_samples in the zsw_history_t to allow the user to know how many of the samples are actually valid. Since the num param today is always the max number of samples. Or can num be changed to actually reflect the number of valid samples?

Because in the battery app now it plots all samples, which are all 0 so it's hard to see anything.

I think we should keep num (but rename it to length) to have an indicator about the maximum length. Then we add a sample counter. Or we can use write_index for it?

Sounds good, or even max_samples or sample_buffer_size so it's clear it's a different length

@Kampi Kampi force-pushed the Add-Generic-History branch 5 times, most recently from f0e3563 to 2b2756e Compare May 22, 2024 19:44
@jakkra jakkra marked this pull request as ready for review May 23, 2024 05:52
Kampi and others added 7 commits May 23, 2024 10:53
@Kampi @jakkra
Add generic history
f0df7ba
@Kampi @jakkra
Rework history to support the Zephyr settings API
30ad566
@Kampi @jakkra
mend
eed7412
@Kampi @jakkra
- Fix wrong assert macro usage
848606b 
- Replace assert with Zephyr assert
@Kampi @jakkra
- Fix wrong load callback
cea342f
@Kampi @jakkra
- Use battery sampling time from Kconfig
0fef00a 
- Set sampling time for battery to 1 minute for POSIX build
- Add log level for battery to Kconfig
- Add random battery levels for POSIX build
- Move update time for the battery application to Kconfig
@Kampi @jakkra
- zsw_history_save is now using the write index as a length indicator…
a166dba 
… and not the total buffer size to save the data (otherwise the empty part of the buffer will be saved)

- zsw_history_del is now deleting the history from NVS
- Rename "num" in history object to "max_samples"
- Add zsw_history_samples to get the number of samples stored in the buffer
- zsw_history_load does not delete the settings anymore
- zsw_history_save does not add a sample to the history anymore
- Exclude battery app source files when battery app should not be included
jakkra
jakkra requested changes May 23, 2024
View reviewed changes
app/src/history/zsw_history.c Outdated Show resolved Hide resolved
app/src/history/zsw_history.c Outdated Show resolved Hide resolved
@Kampi Kampi requested a review from jakkra May 24, 2024 17:31
@jakkra
Copy link
Owner

jakkra commented May 24, 2024

Not working yet

@Kampi
Copy link
Collaborator Author

Kampi commented May 24, 2024

What's the issue?

@jakkra
Copy link
Owner

jakkra commented May 24, 2024 •
edited

What's the issue?

I thunk the get sample function I did not get correct.
IMG20240524204938

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

@jakkra jakkra Awaiting requested review from jakkra

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
ZSWatch
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Kampi @jakkra