fsfw/fsfw
fsfw/fsfw
10
3
Fork 6
Code Issues 32 Pull Requests 4 Releases 10 Wiki Activity

mueller_stopwatch #30

Merged
muellerr merged 18 commits from KSat/fsfw:mueller_stopwatch into master 2020-07-07 12:05:04 +02:00
Conversation 3 Commits 18 Files Changed 7 +179 -31
muellerr commented 2020-04-08 19:34:32 +02:00
Owner
Copy Link

Fixes #29 . Was tested with milliseconds and seconds output.

Fixes #29 . Was tested with milliseconds and seconds output.
muellerr commented 2020-04-17 14:10:54 +02:00
Author
Owner
Copy Link

This is introduced in #29

This is introduced in #29
muellerr added the
feature
 label 2020-04-30 22:01:34 +02:00
gaisser reviewed 2020-05-05 18:58:50 +02:00
gaisser left a comment
Owner
Copy Link

Added a few comments.I'm not an expert in the Timekeeper class, so i'm not sure why getTicks was added

Added a few comments.I'm not an expert in the Timekeeper class, so i'm not sure why getTicks was added
osal/FreeRTOS/Timekeeper.cpp
@ -1,20 +1,24 @@
#include "Timekeeper.h"
#include <FreeRTOSConfig.h>
/**
gaisser commented 2020-05-05 17:47:29 +02:00
Owner
Copy Link

Where does this header comm from?

Where does this header comm from?
muellerr commented 2020-05-05 19:13:24 +02:00
Author
Owner
Copy Link

I don't know. TickType_t is defined in partmacro.h (includes by FreeRTOS.h, just portmacro.h did not work I think) and xGetTickCount is part of task.h .

I don't know. TickType_t is defined in partmacro.h (includes by FreeRTOS.h, just portmacro.h did not work I think) and xGetTickCount is part of task.h .
muellerr commented 2020-05-29 17:52:58 +02:00
Author
Owner
Copy Link

I now know where it comes from: The configTICK_RATE_HZ is used and is located
inside the FreeRTOSConfig.h file. This file needs to be in the include path in any case, but compiler/indexer warnings are always annoying, so I included it explicitely.

I now know where it comes from: The configTICK_RATE_HZ is used and is located inside the FreeRTOSConfig.h file. This file needs to be in the include path in any case, but compiler/indexer warnings are always annoying, so I included it explicitely.
timemanager/Clock.h Outdated
@ -8,3 +8,3 @@
#include <framework/globalfunctions/timevalOperations.h>
typedef uint32_t millis_t;
gaisser commented 2020-05-05 17:47:51 +02:00
Owner
Copy Link

Where are does types used?

Where are does types used?
muellerr commented 2020-05-05 19:14:01 +02:00
Author
Owner
Copy Link

stopwatch. could also be used somewhere else, is more explicit in my opinion. maybe also use it instead of uint32_t for all milliseconds related stuff?

stopwatch. could also be used somewhere else, is more explicit in my opinion. maybe also use it instead of uint32_t for all milliseconds related stuff?
gaisser commented 2020-05-07 17:31:50 +02:00
Owner
Copy Link

Hm I think we should stick with timeval and the types defined there. Someday this might be replaced by timespec and uint32_t will be to small anyway.

Hm I think we should stick with timeval and the types defined there. Someday this might be replaced by timespec and uint32_t will be to small anyway.
timemanager/Clock.h Outdated
@ -9,2 +9,3 @@
typedef uint32_t millis_t;
typedef float seconds_t;
gaisser commented 2020-05-05 17:48:33 +02:00
Owner
Copy Link

Seconds are converted to double in timevalOperations::toDouble(elapsedTime), this would not fit to the float type.

Seconds are converted to double in timevalOperations::toDouble(elapsedTime), this would not fit to the float type.
muellerr commented 2020-05-05 19:14:15 +02:00
Author
Owner
Copy Link

second is double now

second is double now
timemanager/Stopwatch.cpp
@ -0,0 +24,4 @@
return elapsedTime.tv_sec * 1000 + elapsedTime.tv_usec / 1000;
}
seconds_t Stopwatch::stopSeconds() {
gaisser commented 2020-05-05 18:57:40 +02:00
Owner
Copy Link

Here a float is returned but a double is calculated.

Here a float is returned but a double is calculated.
gaisser reviewed 2020-05-07 17:32:19 +02:00
timemanager/Stopwatch.h
@ -0,0 +30,4 @@
* object destruction
* @param displayMode Display format is either MS rounded or MS as double
* format
* @param outputPrecision If using double format, specify precision here.
gaisser commented 2020-05-07 17:29:20 +02:00
Owner
Copy Link

The last part of the comment is outdated

The last part of the comment is outdated
muellerr commented 2020-05-07 20:01:14 +02:00
Author
Owner
Copy Link

merged upstream master, doc corrected.

merged upstream master, doc corrected.
muellerr commented 2020-05-29 17:40:11 +02:00
Author
Owner
Copy Link

UPDATE: The linux version does not compile because a static Clock.h function was not implemented in linux. Another problem is that the getUptime functions from Linux only provide seconds accuracy right now. Therefore, i will replace these calls by getClock_timeval() which should provide milliseconds accuracy for all systems.

UPDATE: The linux version does not compile because a static Clock.h function was not implemented in linux. Another problem is that the getUptime functions from Linux only provide seconds accuracy right now. Therefore, i will replace these calls by getClock_timeval() which should provide milliseconds accuracy for all systems.
muellerr reviewed 2020-05-29 17:53:57 +02:00
osal/linux/Clock.cpp
@ -65,6 +65,15 @@ ReturnValue_t Clock::getClock_usecs(uint64_t* time) {
return HasReturnvaluesIF::RETURN_OK;
}
timeval Clock::getUptime() {
muellerr commented 2020-05-29 17:53:57 +02:00
Author
Owner
Copy Link

this static function was missing.

this static function was missing.
gaisser commented 2020-06-02 14:34:54 +02:00
Owner
Copy Link

Ah great - That was quite a big bug.

Ah great - That was quite a big bug.
gaisser reviewed 2020-06-02 14:43:26 +02:00
timemanager/Clock.h Outdated
gaisser commented 2020-06-02 14:43:26 +02:00
Owner
Copy Link

I still don't think that those typedefs are a good idea. Maybe we should stick to the types timeval or timespec uses or even use the types C++11 uses in duration Link.

I still don't think that those typedefs are a good idea. Maybe we should stick to the types timeval or timespec uses or even use the types C++11 uses in duration [Link](https://www.cplusplus.com/reference/chrono/duration/).
muellerr reviewed 2020-06-02 20:52:46 +02:00
timemanager/Clock.h Outdated
muellerr commented 2020-06-02 20:52:46 +02:00
Author
Owner
Copy Link

Hmmm.. it just makes it a bit mor explicit. I guess you mean for example time_t ?

These typedefs were inteded to explicitely be used everywhere uint32_t is used to pass around millisecond values.

The new C++ types are a bit more complicated but the clock library will become more powerful soon anyway. I started to play around with the chrono library.
Maybe continue using uint32_t for now?

Hmmm.. it just makes it a bit mor explicit. I guess you mean for example time_t ? These typedefs were inteded to explicitely be used everywhere uint32_t is used to pass around millisecond values. The new C++ types are a bit more complicated but the clock library will become more powerful soon anyway. I started to play around with the chrono library. Maybe continue using uint32_t for now?
muellerr reviewed 2020-06-02 22:02:17 +02:00
timemanager/Clock.h Outdated
muellerr commented 2020-06-02 22:02:17 +02:00
Author
Owner
Copy Link

instead of writing something like uint32_t lockTimeout, I could write millis_t lockTimeout for example

instead of writing something like uint32_t lockTimeout, I could write millis_t lockTimeout for example
gaisser self-assigned this 2020-06-25 15:26:43 +02:00
muellerr closed this pull request 2020-07-07 12:05:04 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
API Change

Changes the API, so Users need to adapted

  
Breaking API Change

Changes the API in a non backwards compatible way

  
bug

Something is not working

  
build

Issues related to the buildsystem, including CI

  
cosmetics

Small change most users won't notice

  
Documentation

Improvement of code documentation

  
duplicate

This issue or pull request already exists

  
feature

New feature

  
help wanted

Need some help

  
hotfix

Quick or important fix that is merged directly into master, causing a new revision

  
invalid

#worksforme

  
question

More information is needed

  
Refactor

Contains a refactoring of some code

  
Tests

Issues related to testing code

  
wontfix

This won't be fixed

No Label
API Change
Breaking API Change
bug
build
cosmetics
Documentation
duplicate
feature
help wanted
hotfix
invalid
question
Refactor
Tests
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
gaisser
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: fsfw/fsfw#30
No description provided.
Delete Branch "KSat/fsfw:mueller_stopwatch"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?