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

VT change string value #231

Open
bjoernpoettker opened this issue Apr 1, 2023 · 11 comments
Open

VT change string value #231

bjoernpoettker opened this issue Apr 1, 2023 · 11 comments
Assignees
Labels
investigating Looking into this issue / need more info iso: virtual terminal Related to the ISO-11783:7 standard

Comments

@bjoernpoettker
Copy link

bjoernpoettker commented Apr 1, 2023

Describe the bug
I can't change the value of a string input or string output field

Supporting Documentation
I'm trying to change the value of an string field with send_change_string_value it doesn't work. The function to change a number value of an numeric field work.

TestVirtualTerminalClient->send_change_string_value(OutputString_11000, static_cast<std::string>("hello"));

the function returns true.

Environment (please complete the following information):

  • OS: win 10
  • CAN Driver innomaker
@bjoernpoettker bjoernpoettker added the investigating Looking into this issue / need more info label Apr 1, 2023
@GwnDaan
Copy link
Member

GwnDaan commented Apr 1, 2023

Hi! I saw this exact behavior recently as well. Back then I thought it was my object pool that was wrong somehow. But since you're also facing it, I think send_change_string_value is indeed not working, which is not surprising because it has never been tested before afaik.

I can do some research probably later today to find the issue behind this, I'll keep you updated 😄

@ad3154
Copy link
Member

ad3154 commented Apr 1, 2023

Keep in mind too that changing a string value has a lot of mysterious caveats to it in the standard.

  • If you try to change a string value to a value longer than the original length of the string in the object pool, it will fail when the VT server processes it. Padding a string in the pool with spaces can help mitigate this.
  • If you change the string to be shorter than the original object pool length, the VT may permanently truncate the length of that string. Again, padding with spaces is required to avoid this.
  • Input strings generally do not support the change string value command, you'd need to have a string variable object reference and change that value of the string variable.
  • Changing the value of an output string only works if its variable reference is the null object ID (0xFFFF)

@GwnDaan
Copy link
Member

GwnDaan commented Apr 1, 2023

My results after testing send_change_string_value with a slightly modified version of VT3ExampleTarget:

It works fine! However, changing 2 string values at once is not possible right now, due to each command using a TP session. And we don't queue these TP sessions, hence the send_change_string_value will return false when trying to send a second time immediately after sending the first one.

I tested both combinations of output strings, one is using the value attached to the OutputString object itself, and the other is using a StringVariable in combination with the OutputString.

The modified example can be found here: change_string_value_example.zip

@ad3154
Copy link
Member

ad3154 commented Apr 1, 2023

It works fine!

Sweet!

we don't queue these TP sessions

Yeah, we also don't really want to queue TP sessions, as that could rapidly bloat our RAM usage.
This is why we include this little helper class: https://github.com/ad3154/Isobus-plus-plus/blob/main/utility/include/isobus/utility/processing_flags.hpp

If you set up a set of flags with this little helper class and service it appropriately, you can easily and statelessly process the desired state of any VT object, and automatically re-try to send messages when the interface is busy.

I have been meaning to expand the VT example for some time to include this functionality as an example, so I'll create another issue to demo some more complex VT interactions in the example using this method.

@GwnDaan
Copy link
Member

GwnDaan commented Apr 3, 2023

Yeah, we also don't really want to queue TP sessions

Hey @ad3154, can you help me out here? I just saw CANNetworkConfiguration provides set_max_number_transport_protcol_sessions:
"Configures the max number of concurrent TP sessions to provide a RAM limit for TP sessions". This value is set to 4 by default, doesn't that mean that sending 2 change_string_value commands at once should in theory be possible? Each command should only use 1 TP session right?

@ad3154
Copy link
Member

ad3154 commented Apr 3, 2023

According to the standard, you can only have 1 connection mode TP session with a certain PGN active between 2 control functions at a time, and since two change_string_value calls use the same PGN, they cannot happen at the same time. In theory, if you send 2 TP messages using different PGNs it should work simultaneously up to that limit of 4 by default. Likewise, if you sent same change string value command to 2 different VTs at the same time, that too should work up to that limit.

@ad3154
Copy link
Member

ad3154 commented Apr 3, 2023

So I guess what I'm saying is that we restrict it such that only 1 session can exist with identical control functions and PGN to avoid breaking the standard. In theory we could re-purpose some of those sessions to queue identical sessions, but I would advocate instead for making the application side more tolerant of retries and transient issues on the bus, because otherwise it just pushes the underlying problem to be 4 change_string_value calls break it instead of one. Likewise, getting close to that session cap may block other protocols from getting out time-sensitive messages. I hope to clarify how easy this could be to do in the app in #234

@bjoernpoettker
Copy link
Author

Hi! I changed my code a little bit and implemented the flags, but i didn't know if I use it the right way...

I am processing my flags in the following function:

void processFlags(std::uint32_t flag, void *parentPointer)
{
    auto currentFlag = static_cast<UpdateVTStateFlags>(flag);
    bool transmitSuccessful = false;

    if (vtClient->get_is_connected())
    {
        switch (currentFlag)
        {
        case UpdateVTStateFlags::hoeheAn_VarNum:
        {
            transmitSuccessful = vtClient->send_change_numeric_value(InputNumber_9000, switchOnHeight);
        }
        break;

        case UpdateVTStateFlags::hoeheAus_VarNum:
        {
            transmitSuccessful = vtClient->send_change_numeric_value(InputNumber_9001, switchOffHeight);
        }
        break;

        case UpdateVTStateFlags::hoeheAktuell_VarNum:
        {
            transmitSuccessful = vtClient->send_change_numeric_value(OutputNumber_12000, actualHeight);
        }
        break;

        case UpdateVTStateFlags::status_VarStr:
        {
            if (relayStatus == 0)
                transmitSuccessful = vtClient->send_change_string_value(StringVariable_22000, static_cast<std::string>("aus       "));
            else
                transmitSuccessful = vtClient->send_change_string_value(StringVariable_22000, static_cast<std::string>("an        "));
        }
        break;

        case UpdateVTStateFlags::manuellSoftkey_Enabled:
        {
            if (modus == 0)
                transmitSuccessful = vtClient->send_enable_disable_object(SoftKey_5001, isobus::VirtualTerminalClient::EnableDisableObjectCommand::EnableObject);
            else
                transmitSuccessful = vtClient->send_enable_disable_object(SoftKey_5001, isobus::VirtualTerminalClient::EnableDisableObjectCommand::DisableObject);
        }
        break;

        default:
        {
        }
        break;
        }
    }

    if (!transmitSuccessful)
        txFlags->set_flag(flag);
}
  • Is it the right to set the flag when the transmission failed?
  • I can't set a softkey to disabled - is this right?

@ad3154
Copy link
Member

ad3154 commented Apr 14, 2023

Hey! This seems essentially correct, just make sure processFlags is called cyclically.

Take a look at the example I'm working on here for an early peek at how I'm doing it if you'd like (though it won't compile until #235 and #237 are merged).

You may also want to set all the flags at initialization time to ensure the object pool's state is well defined, as well as consider optionally implementing rudimentary filtering on if an object is shown.

Is it the right to set the flag when the transmission failed?

Yes! This is how it re-tries if the stack is busy next time processFlags is called. It's also useful for persisting the idea that a value needs to be updated if you're doing filtering on if an object is shown before actually sending the message.

I can't set a softkey to disabled - is this right?

Correct, a key object as defined in ISO11783-6 doesn't have an enabled attribute or option, unlike a button (and even for buttons it only works for version 4+). Keys only have a background colour, key code, macros, and a set of child objects. If you want to get a similar effect, there are some options, such as:

  • Place an object pointer in the soft key mask and set it to the null object (65535) to make it disappear! Then set it back to your key object to make it reappear.
  • Place an object pointer on the key itself and swap the appearance of the key to some disabled appearance using other objects like picture graphics, and store the enable/disable state in your application.
  • Make a different soft key mask with that key removed and swap between them

@GwnDaan
Copy link
Member

GwnDaan commented Nov 10, 2023

@ad3154 Do we want to document any of these questions in our FAQ? Or can we close this do you think?

@GwnDaan GwnDaan added the iso: virtual terminal Related to the ISO-11783:7 standard label Nov 10, 2023
@ad3154
Copy link
Member

ad3154 commented Nov 10, 2023

Adding an FAQ entry for "Why do some calls to send_something in the VT client fail sometimes" could be a good thing to add where we could document a workflow for retrying messages as well as why TP causes it. I wonder if we could integrate retries automatically by optionally wrapping the VT client in another class that managed retries automatically so that people wouldn't have to implement their own processFlags function... I will have to think on that....

I also think it would be nice to have a page that describes what you can do with all the different VT objects, such as enable/disable, but I think that might be crossing a line about the ISO documents's copyright 😣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigating Looking into this issue / need more info iso: virtual terminal Related to the ISO-11783:7 standard
Projects
None yet
Development

No branches or pull requests

3 participants