Skip to content
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

Plant Sensor line connects to wrong inertia body. #456

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ldwoolley
Copy link

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)

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants