search

mtiller / ModelicaBook

Source for my new Modelica Book
https://mbe.modelica.university
Other
95 stars 68 forks source link

Plant Sensor line connects to wrong inertia body. #456

Open ldwoolley opened 1 year ago

ldwoolley commented 1 year ago

The model diagram view showed the sensor connected to inertia2.flange_a, however the text view declared the connection connected to inertia1.flange_a. The output was correct and matched the description in the training description, but the image sent me trying to figure out why the speed of inertia2 did not match the speed of the sensor. I have proposed a correction to the connection and moved the sensor up to separate the lines from the components.

Thanks for the training tool. It has been most informative. I am following the class on OpenModelica Connection Editor 1.19.2 (64-bit)

ldwoolley commented 1 year ago

I have found that the same issue exists in PlantWithSampleHold for the SampleHold sensor, PlantWithIntervalMeasure for the IntervalMeasure sensor and PlantWithPulseCounter for the PulseCounter sensor. I am happy to make the corrections so that the models are consistent. Currently, the models connect the sensors to inertia1.flange_a and inertia1.flange_b, however the connector path goes to inertia2.flange_a and inertia2.flange_b. We can update the connection paths to inertia1, or update the connection declarations to inertia2. Updating the path to inertia1, means the output graphs are correct, but the model images need to update. Updating the declaration to inertia2 means the models are correct, and the output diagrams need to updated. Then I will expand this pull request to include all 4 example models and the document model images.

dietmarw commented 1 year ago

I wonder what the intended connection for the speed sensor is in those examples. It is definitely a mix-up and in the case of the Plant model plainly wrong. So, I would erase the existing connections to the sensor and reconnect them so that the graphics and the connect statements are in line. The question is only should the sensor show the speed of inertia1 or intertia2, @mtiller?

ldwoolley commented 1 year ago

I personally think the inertia1 rotational speed is the more interesting of the two. I have tossed in an image of the two for reference. image

ldwoolley commented 1 year ago

Any thought on this one? I am willing to make the changes and submit the pull request. Just need a decision on the direction that is preferred.