geez0x1 / CompliantJointToolbox

Compliant Joint Toolbox (CJT) for MATLAB and Simulink
GNU General Public License v3.0
37 stars 14 forks source link

Documentation issue in linear models #32

Closed geezIIT closed 6 years ago

geezIIT commented 7 years ago

The linear models list (in-line code documentation) the input as [tau_m, tau_e], while in fact they are [i, tau_e], i.e. current inputs.

This may also exist in other places.

joernmalzahn commented 7 years ago

Ok, let's discuss what we wish to have there. Having a consistent torque interface to the mechanical subsystem models would make sense in my opinion.

This way the electrical subsystems that are actually the torque generators could have voltage as input and torque as output (before or after gearbox, i.e. where should the transmission ratio enter the game?).

joernmalzahn commented 7 years ago

Documentation issue in linear models

geez0x1 commented 6 years ago

I'm currently working on this in the dev_mechanical_torque_inputs branch.

I agree that having a mechanical torque input is more consistent. As for the jointModel block itself, changing its input to mechanical torque makes current limits weird to implement, so I think those should also move to the electrical model blocks. The delay and noise can stay (useful when not using an electrical model), but be reformulated in terms of torque.

The electrical model blocks could have two outputs; current and torque (easier for debugging purposes). Torque should stay after the gearbox; everywhere k_t and n are together, and the mechanical models are defined with all variables reflected across the gearbox.

Things that need updating:

geez0x1 commented 6 years ago

@joernmalzahn Check out the branch if you have time. I think it is a nice improvement. If you have thoughts let me know otherwise I'll merge it into master sometime soon.

geez0x1 commented 6 years ago

@joernmalzahn Have you looked at this? I'm ready to merge it into master.

joernmalzahn commented 6 years ago

Sorry, didn't manage before the holidays. So far it looks good to me.

Only jointBuilderTest/testElectricalSubsystem fails. Can you look at it?

It would be good to have also a couple of tests for the new input.

joernmalzahn commented 6 years ago

The error ocurrs in line 277. verifyTrue failed. --> The value must evaluate to "true".

geez0x1 commented 6 years ago

Hmm, odd. I did go over the tests earlier and updated them. Perhaps I missed this one somehow. Anyway, it's fixed.

I agree more tests are needed. I don't have the time for them now though, perhaps next month. Merging this into master now.