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

VDA5050 Schema Version Not Consistent #43

Open
bobbleballs opened this issue Jul 7, 2023 · 3 comments
Open

VDA5050 Schema Version Not Consistent #43

bobbleballs opened this issue Jul 7, 2023 · 3 comments
Assignees

Comments

@bobbleballs
Copy link
Contributor

Hello, I've been using the VDA5050 Controller with openTCS' VDA5050 Master Controller found here
As I've been doing so I've noticed that there are some inconsistencies with the VDA5050 version that is being targeted.
I've seen that in OrderState.msg that vda5050_msgs/Info[] informations has the 's' on the end.
And the def get_vda5050_mqtt_topic( manufacturer, serial_number, topic, interface_name="uagv", major_version="v1" ): would suggest that V1.1 is the target of this software.
However you also have things like CurrentAction.msg's ActionState enums including PAUSED that only came in V2.0:

# Enums for actionStatus
string WAITING="WAITING"
string INITIALIZING="INITIALIZING"
string RUNNING="RUNNING"
string PAUSED="PAUSED"
string FINISHED="FINISHED"
string FAILED="FAILED"

And the constant DEFAULT_PROTOCOL_VERSION = "2.0.0" suggests that 2.0 is default.

What is the target version of this software? And can this be detailed in the documentation and the schema verified with the standard.

Thanks.

@leandropineda
Copy link
Member

Hey @bobbleballs

Yeah, as discussed on #42 there are some inconsistencies that were inherited from the msgs package we based this project on. I'm sure there are more but fortunately are easy to fix.

I agree major_version=v1 does indeed suggest v1 is the target version, but in fact we tried to make a single connector capable of supporting both version, as opposed to having two separate branches for v1 and v2. This was not fully implemented though -- that's the reason why the constant DEFAULT_PROTOCOL_VERSION is hardcoded and not used on get_vda5050_mqtt_topic for example.

See one related hack as reference: https://github.com/inorbit-ai/ros_amr_interop/blob/humble-devel/vda5050_connector/vda5050_connector_py/mqtt_bridge.py#L165

    # HACK: VDA5050 instantActions message schema differs from v1 to v2. In particular,
    # the ``instantActions`` field from v1 has been renamed to ``actions`` on v2.
    is_v1 = "instant_actions" in vda_instant_action.keys()

@bobbleballs
Copy link
Contributor Author

@leandropineda Ok that explains things, thanks for clearing it up :)

I would suggest that getting the DEFAULT_PROTOCOL_VERSION from the vda5050_connector.py as a constant in the mqtt_bridge is not great as this is actually a parameter and can be changed at run time, so behaviour between the 2 nodes could be different unintentionally if this is changed on the connector params. Maybe adding a parameter to the mqtt_bridge as well with the same default as the vda5050_connector.py that should be set in the launch params file?

Current behaviour is to claim to be running a default version of 2.0.0 but in the get_vda5050_mqtt_topic calls there is no specification so it defaults to v1. This is breaking the standard so should be fixed, as it dumps v2.0.0 messages on a v1 mqtt topic. A fix could be to work out whether it is v1 or v2 from the parameters passed through as detailed above.

@leandropineda
Copy link
Member

Maybe adding a parameter to the mqtt_bridge as well with the same default as the vda5050_connector.py that should be set in the launch params file?

I agree 100%.

I'll leave this issue open as a reminder and to reference the PR once fixed.

@leandropineda leandropineda self-assigned this Aug 9, 2023
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

No branches or pull requests

2 participants