-
Notifications
You must be signed in to change notification settings - Fork 221
Initial FTDI RDM support #1541
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.
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
base: master
Are you sure you want to change the base?
Initial FTDI RDM support #1541
Changes from all commits
bfad65f
78cc4b9
e1869e3
2b5a5fa
3b6cb83
c51928e
10fc5a0
1bdf897
71bd693
8553598
81d120b
8914f56
9e95824
ff09297
851401b
1d09fe1
b02363b
c87ebf6
0141ad0
2373f60
2198e3d
65e11d8
d719f6b
5645a01
a0e6cb4
0ba5dfd
29b32d0
f451f19
abdf4d5
b080a84
9874f6d
ab0e961
4b72f70
17a40b1
532c8c3
d28d3df
92729c4
5f6187e
1946005
a4cfe30
faa98ca
5b150c1
fb51664
5564a1e
d533789
6309300
4553fe3
591234c
3f60c66
7d11c58
f7489b6
c4d0ee3
c8e010f
7bf7ea5
6f84568
f144552
6b08435
b6d41af
0a4a51e
b084dcf
b489f96
8d9691d
1e004ac
c222943
8410da0
6252026
3c5a2e6
4e285cb
c912c7f
a90c1ba
c640e4e
8b9ff70
c2083af
717462d
7b0ecf8
ed1eb6b
5392ab0
c666d71
249d826
bfc14db
48d1dbe
a198e63
368a2f1
e80f64d
011f591
b1dc145
a0d202a
4566ff8
c85f065
e48cebd
14a4513
117f831
1fb4bda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -246,3 +246,7 @@ javascript/new-src/node_modules | |
| .cproject | ||
| .settings | ||
| .vscode/ | ||
| *.config | ||
| *.creator* | ||
| *.files | ||
| *.includes | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,8 @@ | |
| #include <sstream> | ||
| #include <string> | ||
|
|
||
| #include "ola/Logging.h" | ||
|
|
||
| namespace ola { | ||
|
|
||
| using std::string; | ||
|
|
@@ -129,6 +131,11 @@ int64_t BaseTimeVal::InMilliSeconds() const { | |
| m_tv.tv_usec / ONE_THOUSAND); | ||
| } | ||
|
|
||
| int64_t BaseTimeVal::InMicroSeconds() const { | ||
| return (m_tv.tv_sec * static_cast<int64_t>(USEC_IN_SECONDS) + | ||
| m_tv.tv_usec); | ||
| } | ||
|
|
||
| int64_t BaseTimeVal::AsInt() const { | ||
| return (m_tv.tv_sec * static_cast<int64_t>(USEC_IN_SECONDS) + m_tv.tv_usec); | ||
| } | ||
|
|
@@ -272,4 +279,89 @@ void MockClock::CurrentTime(TimeStamp *timestamp) const { | |
| *timestamp = tv; | ||
| *timestamp += m_offset; | ||
| } | ||
|
|
||
| Sleep::Sleep(std::string caller) : | ||
| m_caller(caller) { | ||
| } | ||
|
|
||
| /** | ||
| * @brief Set wanted granularity for usleep and check it. | ||
| * @note does not check at the nanosecond level, | ||
| * since internal sturtures use usecs. | ||
| * | ||
| * @param wanted wanted/needed granularity in usecs | ||
| * @param maxDeviation max deviation in usecs tolerated by calling thread. | ||
| * | ||
| * @attention the granularity of sleep is highly fluctuating depending on the | ||
| * load of the system, a prior GOOD state is no guarantee for future proper | ||
| * timing. | ||
| */ | ||
| bool Sleep::CheckTimeGranularity(uint64_t wanted, uint64_t maxDeviation) { | ||
| TimeStamp ts1, ts2; | ||
| Clock clock; | ||
|
|
||
| m_wanted_granularity = wanted; | ||
| m_max_granularity_deviation = maxDeviation; | ||
|
|
||
| timespec t; | ||
| t.tv_sec = wanted / USEC_IN_SECONDS; | ||
| t.tv_nsec = (wanted % USEC_IN_SECONDS) * ONE_THOUSAND; | ||
|
|
||
| clock.CurrentTime(&ts1); | ||
| this->usleep(1); | ||
| clock.CurrentTime(&ts2); | ||
| TimeInterval interval = ts2 - ts1; | ||
| m_clock_overhead = interval.InMicroSeconds(); | ||
|
|
||
| clock.CurrentTime(&ts1); | ||
| this->usleep(t); | ||
| clock.CurrentTime(&ts2); | ||
|
|
||
| interval = ts2 - ts1; | ||
| m_granularity = (interval.InMicroSeconds() > | ||
| (wanted + maxDeviation + m_clock_overhead)) ? BAD : GOOD; | ||
|
|
||
| OLA_INFO << "Granularity for OlaSleep in " << m_caller << " is " | ||
| << ((m_granularity == GOOD) ? "GOOD" : "BAD") | ||
| << " Requested: " << wanted << " Got: " << interval.InMicroSeconds() | ||
| << " Overhead: " << m_clock_overhead; | ||
| if (m_granularity == GOOD) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| void Sleep::usleep(TimeInterval requested) { | ||
| timespec req; | ||
| req.tv_sec = requested.Seconds(); | ||
| req.tv_nsec = requested.MicroSeconds() * ONE_THOUSAND; | ||
|
|
||
| this->usleep(req); | ||
| } | ||
|
|
||
| void Sleep::usleep(uint32_t requested) { | ||
| timespec req; | ||
| req.tv_sec = requested / USEC_IN_SECONDS; | ||
| req.tv_nsec = (requested % USEC_IN_SECONDS) * ONE_THOUSAND; | ||
| req.tv_sec = 0; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this assigned and then reset?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect an error on my part, though I also have a suspicion that maybe I did not want this function to exceed 1s |
||
|
|
||
| this->usleep(req); | ||
| } | ||
|
|
||
| void Sleep::usleep(timespec requested) { | ||
| timespec rem; | ||
|
|
||
| if (nanosleep(&requested, &rem) < 0) { | ||
| if (errno == EINTR) { | ||
| while (rem.tv_nsec > 0 || rem.tv_sec > 0) { | ||
| requested.tv_nsec = rem.tv_nsec; | ||
| requested.tv_sec = rem.tv_sec; | ||
| nanosleep(&requested, &rem); | ||
| } | ||
| } else { | ||
| OLA_WARN << "nanosleep failed with state: " << errno; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| } // namespace ola | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,13 +152,16 @@ bool StringToBoolTolerant(const string &value, bool *output) { | |
| return false; | ||
| } | ||
|
|
||
| bool StringToInt(const string &value, unsigned int *output, bool strict) { | ||
| bool StringToInt(const string &value, | ||
| unsigned int *output, | ||
| bool strict, | ||
| uint8_t base) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please can we have some new tests for the base parameter.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specifically, you're assuming, and the spec seems to imply, that it's case insensitive, but we should have a test of that so we catch issues on new platforms.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean by tests?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Like the stuff in https://github.com/OpenLightingProject/ola/blob/master/common/utils/StringUtilsTest.cpp#L541-L564 for example. So passing in something in base 36, and also validating that 123ABC converts to the same value as 123abc. |
||
| if (value.empty()) { | ||
| return false; | ||
| } | ||
| char *end_ptr; | ||
| errno = 0; | ||
| long long l = strtoll(value.data(), &end_ptr, 10); // NOLINT(runtime/int) | ||
| long long l = strtoll(value.data(), &end_ptr, base); // NOLINT(runtime/int) | ||
| if (l < 0 || (l == 0 && errno != 0)) { | ||
| return false; | ||
| } | ||
|
|
@@ -168,10 +171,12 @@ bool StringToInt(const string &value, unsigned int *output, bool strict) { | |
| if (strict && *end_ptr != 0) { | ||
| return false; | ||
| } | ||
|
|
||
| *output = static_cast<unsigned int>(l); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has no protection against truncating (which I think was deliberate for your use, but isn't good for the rest of the codebase) Sorry, I meant can you add a long long version of StringToInt and use it for your particular case. Then you can do that particular cast (or some sort of binary mask or whatever) in your specific code, rather than in the general function.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's still returning false, a user who cares is anyhow checking for true/false.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But if we've returned false we shouldn't bother setting the value. |
||
|
|
||
| if (l > static_cast<long long>(UINT32_MAX)) { // NOLINT(runtime/int) | ||
| return false; | ||
| } | ||
| *output = static_cast<unsigned int>(l); | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -199,16 +204,19 @@ bool StringToInt(const string &value, uint8_t *output, bool strict) { | |
| return true; | ||
| } | ||
|
|
||
| bool StringToInt(const string &value, int *output, bool strict) { | ||
| bool StringToInt(const string &value, int *output, bool strict, uint8_t base) { | ||
| if (value.empty()) { | ||
| return false; | ||
| } | ||
| char *end_ptr; | ||
| errno = 0; | ||
| long long l = strtoll(value.data(), &end_ptr, 10); // NOLINT(runtime/int) | ||
| long long l = strtoll(value.data(), &end_ptr, base); // NOLINT(runtime/int) | ||
| if (l == 0 && errno != 0) { | ||
| return false; | ||
| } | ||
|
|
||
| *output = static_cast<unsigned int>(l); | ||
|
|
||
| if (value == end_ptr) { | ||
| return false; | ||
| } | ||
|
|
@@ -218,7 +226,7 @@ bool StringToInt(const string &value, int *output, bool strict) { | |
| if (l < INT32_MIN || l > INT32_MAX) { | ||
| return false; | ||
| } | ||
| *output = static_cast<unsigned int>(l); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This returns exactly the same value as BaseTimeVal::AsInt(), any reason we need the second function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is returning msec not usec...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess AsInt is too then; I've just double-checked and the code is identical in both, only the method name changes.