thomas-haslwanter / scikit-kinematics

Python functions for working with 3D kinematics.
Other
129 stars 45 forks source link

docs errors #10

Closed ivan866 closed 5 years ago

ivan866 commented 6 years ago

http://work.thaslwanter.at/skinematics/html/view.html

q = skin.quat.vel2quat(omegas, q0, rate, 'sf')

vel2quat seems to be deprecated, should be calc_quat

ivan866 commented 6 years ago

http://work.thaslwanter.at/skinematics/html/imus.html

imus.analytical argument order differs in some places, better be named arguments consistently

thomas-haslwanter commented 6 years ago

Hi Ivan, I fixed the first problem. With the second I am not sure I understand you correctly: do you mean that the function "kalman" has the arguments in a different order (def kalman(rate, acc, omega, mag)) as analytical? If that is the case, I have fixed it: now the argument sequence in analytical is: def analytical(rate, accMeasured, omega, initialPosition, R_initialOrientation)

Please let me know if you had that in mind, or something else.

ivan866 commented 6 years ago

I meant that the docs on site and the source code have different argument orders, thus yielding an error if developer decides to use positional arguments rather than named.

The docs after your recent edits still don't match the sources, and you broke the interface in case someone used positional arguments.

thomas-haslwanter commented 6 years ago

Is the current version ok? Or do you see any remaining problems?

ivan866 commented 6 years ago

Seems OK. You should also add imus.pos field. Pretty valuable.

thomas-haslwanter commented 6 years ago

Done. Watch out, though, that minute offsets in the accelerometers can lead to erroneous values quickly!

ivan866 commented 6 years ago

calculate_position=True argument to imus.IMU_Base constructor is absent and thus effectively ignored in source 0.6.8 because imus.py, line 235 position value is never used in def _calc_orientation(self).

...minute offsets in the accelerometers...

You mean I can't calculate position by using just angular speed and acceleration if duration of a record is several minutes long?

thomas-haslwanter commented 6 years ago

def __init__(self, in_file = None, q_type='analytical', R_init = np.eye(3), calculate_position=True, pos_init = np.zeros(3), in_data = None ): This is the code for IMU_Base. Is it possible that you have an old version, and should update (currently 6.9) And regarding the position calculation: I don't know your sensors. But my guess would be that the answer is a simple "no". For example, if you can accept an error of +/- 1 m, and you want to record for 10 min, then your accelerometer would need to have a drift of less than 6 micrometer/sec^2

ivan866 commented 6 years ago

http://work.thaslwanter.at/skinematics/html/imus.html “mahony” … Mahony algorithm, using, acc and omega

In fact, Mahony uses mag too.

thomas-haslwanter commented 6 years ago

You are correct there – will be fixed in the docu.

From: Ivan [mailto:notifications@github.com] Sent: Saturday, January 20, 2018 3:47 PM To: thomas-haslwanter/scikit-kinematics scikit-kinematics@noreply.github.com Cc: Haslwanter Thomas Thomas.Haslwanter@fh-linz.at; State change state_change@noreply.github.com Subject: Re: [thomas-haslwanter/scikit-kinematics] docs errors (#10)

http://work.thaslwanter.at/skinematics/html/imus.html “mahony” … Mahony algorithm, using, acc and omega

In fact, Mahony uses mag too.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHubhttps://github.com/thomas-haslwanter/scikit-kinematics/issues/10#issuecomment-359176524, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABdaRUr2YpWqjxsmdoHhrrBBBxvcG9B2ks5tMfxwgaJpZM4REoHF.

thomas-haslwanter commented 5 years ago

Documentation has been updated.