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.

Sign up for GitHub

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

Jump to bottom

Avoid crashes and better warn when getting PROTO #6473

Merged
merged 6 commits into from
Jan 15, 2024
Merged

Avoid crashes and better warn when getting PROTO #6473

merged 6 commits into from
Jan 15, 2024

Conversation

galou
Copy link
Contributor

@galou galou commented Jan 10, 2024

Avoid crashes and better warn when getting the PROTO in WbSupervisorUtilities::getProtoParameterNodeInstance().

Description
I discovered the bug when we used a cache to save nodes in Python. When calling getPose() or getPosition() we got an error message that wb_supervisor_node_get_position() can exclusively be used with Pose (or derived). Now, the error message is Cannot get the PROTO instance for node LED (type LED). plus the old message. This helped me fixing the bug in our code.

Related Issues
None

Tasks
Add the list of tasks of this PR.

  • Translate the strings

Documentation
No documentation change

Additional context
We probably save the result of getMFNode() in Python too soon because the pointed object is not a valid node later in our code (maybe the pointer changes when changing the DEF?).

@galou galou requested a review from a team as a code owner January 10, 2024 14:55
@omichel
Copy link
Member

omichel commented Jan 10, 2024

Can you please fix the source tests which are failing to a wrong coding style?

@galou
Copy link
Contributor Author

galou commented Jan 11, 2024

When at it. Is the log message on failure of getFirstFinalizedProtoInstance() OK?

@omichel
Copy link
Member

omichel commented Jan 11, 2024

When at it. Is the log message on failure of getFirstFinalizedProtoInstance() OK?

I am not sure what is the exact difference between modelName and nodeModelName, can you explain it?

@galou
Copy link
Contributor Author

galou commented Jan 11, 2024

According to the little knowledge I have about Webots, modelName is the name of the node and nodeModelName is its type. We can maybe remove nodeModelName.

@omichel
Copy link
Member

omichel commented Jan 11, 2024

According to the little knowledge I have about Webots, modelName is the name of the node and nodeModelName is its type. We can maybe remove nodeModelName.

Yes, I found the confirmation here. I will propose an improvement to the terminology you used.

@galou
Copy link
Contributor Author

galou commented Jan 11, 2024

FYI, I probably found the cause of the issue that brought me to this PR: #6474 .

Avoid crashes and better warn when getting PROTO
d5e6c1d 
Avoid crashes and better warn when getting the PROTO in
`WbSupervisorUtilities::getProtoParameterNodeInstance()`.

Signed-off-by: Gaël Écorchard <gael@km-robotics.cz>
@omichel omichel added this to the R2023b-rev1 milestone Jan 11, 2024
omichel
omichel previously approved these changes Jan 11, 2024
View reviewed changes
@omichel omichel self-requested a review January 15, 2024 07:30
@omichel omichel merged commit 0dbce8a into cyberbotics:master Jan 15, 2024
20 checks passed
@galou galou deleted the better_warning branch January 16, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omichel omichel omichel approved these changes

Assignees

@galou galou

Labels
None yet
Milestone
R2023b-rev1
Development

Successfully merging this pull request may close these issues.
2 participants
@galou @omichel