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
Data rate fixes #9
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mlehtima
requested changes
Mar 22, 2023
addd853
to
77d7769
Compare
mlehtima
requested changes
Mar 24, 2023
If data rate adjustment is skipped because data rate did not change, setDelay() returns failure - which then causes journal spamming and other hiccups. Return success if data rate adjustment is skipped on purpose. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
The order of interval and session id parameters for setInterval() methods varies within class hierarchy present in sensorfw. Normalize situation so that when setInterval() method takes session id parameter, it is always the first argument to pass - which aligns with dbus interface. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
As a preparatory step for using different time units in D-Bus interface and internally within sensorfw: use "_ms" suffix for all interval arguments and member variables. Also make affected classes follow m_variableName naming convention. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
Using integer millisecond sampling interval is problematic for specifying high acquisition rates as it limits choice to 1000, 500, 333, 250, 200, ... Hz and thus rules out e.g. 400 Hz altogether. Use microsecond granularity interval time internally and provide new setDataRate() method that takes double precision floating point Hertz parameter - while retaining the already existing setInterval() method for the sake of backwards compatibility. Note that this only deals with D-Bus interface level. Further changes in sensorfw side are needed to actually support e.g. 400 Hz data rates. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
Sample intervals within sensorfwd are handled in microsecond granularity and converted to appropriate units when dealing with external interfaces such as D-Bus, Android hal, sysfs controls, or configuration files. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
NodeBase::evaluateIntervalRequests is virtual method that about every derived class overrides - with almost identical copy paste code. The only difference between these variants is how placeholder zero interval is handled. Base behavior seems to want to treat it as "as fast as possible" while others take it to mean "use the defaults." When intervals were defined in millisecond granularity, using zero might have had some use for reaching > 1kHz data rates. Now that microseconds are used, the boundary for expressable data rates has moved to >1MHz level and dual role for zero value is less useful. Change NodeBase behavior to match what the multitude of derived classes used to do and drop all the now unnecessary copy-paste methods. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
What kinds of data rates are available for sensors varies from one device to another. Having sensorfwd completely ignore out of bounds data rate requests creates a situation where applications can work in completely different ways on different devices. Instead of rejecting out of bounds data requests, choose closest supported data rate. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
Declination deals with sensor event timestamps in microseconds. No code changes required, but rename member variables to conform with m_variableName / s_staticVariable convention and add time unit suffixer. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
There is code for tracking buffer sizes and intervals in sessionId based hash tables elsewhere - presumably these node base member variables are left overs from time before those were taken in use? Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
For one or another reason both variableName_ and m_variableName conventions are used in the same class. Normalize to m_variableName style. Signed-off-by: Simo Piiroinen <simo.piiroinen@jolla.com>
77d7769
to
fd8f992
Compare
mlehtima
approved these changes
Mar 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[nodebase] Use closest supported data rate. Fixes JB#60314
What kinds of data rates are available for sensors varies from one
device to another. Having sensorfwd completely ignore out of bounds
data rate requests creates a situation where applications can work in
completely different ways on different devices.
Instead of rejecting out of bounds data requests, choose closest
supported data rate.
[sensorfw] Drop evaluateIntervalRequests overloading. JB#60314
NodeBase::evaluateIntervalRequests is virtual method that about every
derived class overrides - with almost identical copy paste code.
The only difference between these variants is how placeholder zero
interval is handled. Base behavior seems to want to treat it as "as
fast as possible" while others take it to mean "use the defaults."
When intervals were defined in millisecond granularity, using zero
might have had some use for reaching > 1kHz data rates. Now that
microseconds are used, the boundary for expressable data rates has
moved to >1MHz level and dual role for zero value is less useful.
Change NodeBase behavior to match what the multitude of derived
classes used to do and drop all the now unnecessary copy-paste
methods.
[hybrisadaptor] Fix setDelay() return value
If data rate adjustment is skipped because data rate did not change,
setDelay() returns failure - which then causes journal spamming and
other hiccups.
Return success if data rate adjustment is skipped on purpose.
[sensorfw] Handle intervals in microsecond granularity. Fixes JB#60313
Sample intervals within sensorfwd are handled in microsecond granularity
and converted to appropriate units when dealing with external interfaces
such as D-Bus, Android hal, or sysfs control files.
[sensorfw] Add setDataRate() D-Bus method. JB#60417
Using integer millisecond sampling interval is problematic for
specifying high acquisition rates as it limits choice to
1000, 500, 333, 250, 200, ... Hz and thus rules out e.g. 400 Hz
altogether.
Use microsecond granularity interval time internally and provide new
setDataRate() method that takes double precision floating point
Hertz parameter - while retaining the already existing setInterval()
method for the sake of backwards compatibility.
Note that this only deals with D-Bus interface level. Further changes
in sensorfw side are needed to actually support e.g. 400 Hz data rates.
[sensorfw] Explicitly indicate interval time units. JB#60313
As a preparatory step for using different time units in D-Bus interface
and internally within sensorfw: use "_ms" suffix for all interval
arguments and member variables.
Also make affected classes follow m_variableName naming convention.
[sensorfw] Normalize setInterval() parameter order. Fixes JB#60301
The order of interval and session id parameters for setInterval() methods
varies within class hierarchy present in sensorfw.
Normalize situation so that when setInterval() method takes session id
parameter, it is always the first argument to pass - which aligns with
dbus interface.