DSL type conversion documentation is difficult to understand

[Nadahar]

I'm trying to work on #2728 to document some of the new possibilities that have recently been added to DSL rules/scripts. I have to try to figure out where to put the new information in the existing documentation, and I'm having some trouble in that department too. But, this is about one particular section:

The different syntax for the generic and the objective-specific differs and is given in the table below:

Generic (Action)	Specific (Method)
postUpdate(MyItem, new_state)	MyItem.postUpdate(new_state)
sendCommand(MyItem, new_state)	MyItem.sendCommand(new_state)

The benefit of using Objects over primitives is apparent through the following type conversions that are automatically invoked by Object as the context requires.
Using the method MyItems.sendCommand() that is owned by MyItem will use the sendCommand method that is suitable to make the necessary type conversions.
For example, the NumberItem class would have a sendCommand(int), sendCommand(long), sendCommand(float), sendCommand(double), sendCommand(Number), sendCommand(DecimalType), and sendCommand(String) method.
Each of these separate methods is individually written to handle all of these different types of Objects.
MyItem will automatically apply the method that corresponds to the argument type.

I've read it over and over, and I can't get it to make sense. I would think that, when I'm in a position that I can add to the documentation, I should be able to understand what's there. I also question what I think it suggests, that only the extension form supports autoboxing of Number primitives (if that's what it's actually trying to say). I find methods like this in BusEventImpl, which should be able to signature match either "form":

    @Override
    public void sendCommand(Item item, Number command) {
        sendCommand(item, command, null);
    }

Unless Xtext is doing something really weird here, standard Java logic would be that e.g. an int can be autoboxed into Integer, which is a Number, so it will do the autoboxing and then invoke that method.

The explanation seems to come from #483 and be (at least partially) based on this forum thread. I can't however find what I consider "the confusing part" in the forum post.

I don't "have to" understand this section to do what I'm trying to do, I can just leave it be and pretend that everybody else understand it, but I know from experience that if I can't understand something, I'm usually not alone.

@rkoshak Do you have anything to add here, being the author of the "source forum post"?

[rkoshak]

The purpose of this section is to explain why, in rules dsl, it is better to use MyItem.sendCommand(cmd) instead of sendCommand("MyItem", cmd.toString).

In the later case you don't get the benefit of any of the unboxing and all that. At least that used to be the case and I can't imagine it's changed.

[Nadahar]

In the later case you don't get the benefit of any of the unboxing and all that. At least that used to be the case and I can't imagine it's changed.

You don't get automatic boxing if you use sendCommand("MyItem", cmd.toString), because then you have already converted it to a string, but sendCommand("MyItem", cmd) should be "exactly the same" as MyItem.sendCommand(cmd). That said, I agree that the latter form is probably "better" to use as a convention, but the explanation of why this doesn't work, doesn't make sense to me. Also, this:

For example, the NumberItem class would have a sendCommand(int), sendCommand(long), sendCommand(float), sendCommand(double), sendCommand(Number), sendCommand(DecimalType), and sendCommand(String) method.
Each of these separate methods is individually written to handle all of these different types of Objects.

..is plain wrong as far as I can tell. I've looked in NumberItem, and there are no such methods there. It wouldn't have made sense anyway, because of static type resolution and the fact that the script wouldn't "know" that it's a NumberItem unless it had been explicitly cast to that.

I just find the whole section very confusing, and isn't even sure what I'm supposed to "be left with" after reading it. If it's as simple as "use MyItem.sendCommand(cmd), not sendCommand("MyItem", cmd)", I think it would be better to just state that.

[Nadahar]

Maybe a lot actually has changed since this was written, because this also doesn't correspond with what I find in the code:

Actions only support two String arguments as all Objects will support the conversion toString.
sendCommand (MyItem, new_state) will automatically use the MyItem.toString method to convert MyItem into a String.
It will also attempt to do so with the second argument if new_state is not already a String.

It is correct that there is a method that tries to convert a string to a Command:

    @Override
    public void sendCommand(String itemName, String commandString, @Nullable String source) {
        try {
            Item item = itemRegistry.getItem(itemName);
            Command command = TypeParser.parseCommand(item.getAcceptedCommandTypes(), commandString);
            if (command != null) {
                publisher.post(ItemEventFactory.createCommandEvent(itemName, command, buildSource(source)));
            } else {
                logger.warn("Cannot convert '{}' to a command type which item '{}' accepts: {}.", commandString,
                        itemName, getAcceptedCommandNames(item));
            }
        } catch (ItemNotFoundException e) {
            logger.warn("Item '{}' does not exist.", itemName);
        }
    }

..but if what you send is already a Command, there's no need for this conversion, and this is used instead:

    @Override
    public void sendCommand(Item item, Command command, @Nullable String source) {
        if (item != null) {
            publisher.post(ItemEventFactory.createCommandEvent(item.getName(), command, buildSource(source)));
        }
    }

Maybe this was originally all string-based, or has it just been a misunderstanding?

[rkoshak]

You don't get automatic boxing if you use sendCommand("MyItem", cmd.toString),

That;s the whole point.

If you send a String "foo" to a Number Item, using sendCommand(string, strinbg) you won't get an error in your rule. An error will be generated in core.

If you send a String "foo" to a Number Item using MyItem.sendCommand("foo") the errror will be generated from your rule. Pasring and making sure the command is valid for the Item type occurs before the event is places on the event bus.

They are somehow added to the Item class when it's injected into Rules DSL.

I don't know how these methods are added but I bet it's the same way all the persistence actions are added to the Item Object inside the rule. You'll notice none of those are part of the Item class and yet https://openhab-docs-preview.netlify.app/docs/configuration/persistence.html#persistence-extensions-in-scripts-and-rules

It used to be the case that you couldn't send a DecimalType as a command to a Number Item because the Number Item class had both a sendCommand(DecimalType) and a sendCommand(Number) and because DecimalType is a Number Rules DSL would through an "am,biguous method call" exception.

Maybe a lot actually has changed since this was written, because this also doesn't correspond with what I find in the code:

In truth, the Item Object you see actually in the rule has tons more than the Item class would imply.

Maybe something did change but it most definitely was not a misunderstanding.

You cannot only rely on the JavaDocs and Java classes to understand how things work in Rules DSL. It's always been one of the things I hate about it.

[Nadahar]

If you send a String "foo" to a Number Item, using sendCommand(string, strinbg) you won't get an error in your rule. An error will be generated in core.

If you send a String "foo" to a Number Item using MyItem.sendCommand("foo") the errror will be generated from your rule. Pasring and making sure the command is valid for the Item type occurs before the event is places on the event bus.

There's a difference here - the method I was thinking of takes an Item as the first argument, not a String. So, what I tried to say is that MyItem.sendCommand("foo") and sendCommand(MyItem, "foo") should result in the exact same behavior. When the first argument is a String, be "string variant" I showed above will be invoked. When you say that it won't "generate an error in your rule", I'm not entirely sure what it means, but I suspect that this behavior is caused by this part of the method:

        } catch (ItemNotFoundException e) {
            logger.warn("Item '{}' does not exist.", itemName);
        }

What this does is explicitly to not throw an exception if the Item doesn't exist, but just log a warning. The same is done if you specify an incompatible type:

            } else {
                logger.warn("Cannot convert '{}' to a command type which item '{}' accepts: {}.", commandString,
                        itemName, getAcceptedCommandNames(item));
            }

So, the method is explicitly made so that it won't throw an exception, only log it and carry on. It's a strange thing to do IMO, because the rest of the script probably won't work when you just "skip a step". It sounds to me like it's this very behavior that is the reason why avoiding sendCommand(string, string) is recommended. This behavior could very easily be changed so that it actually causes a failure.

They are somehow added to the Item class when it's injected into Rules DSL.

I don't know how these methods are added but I bet it's the same way all the persistence actions are added to the Item Object inside the rule. You'll notice none of those are part of the Item class and yet https://openhab-docs-preview.netlify.app/docs/configuration/persistence.html#persistence-extensions-in-scripts-and-rules

I know, I just added a bunch of these myself with my "DSL helper methods". They are called "extensions", and is actually the basis for why Xtend is called Xtend. They must have thought that this extension mechanism was very brilliant. I can see how it can be handy, but it's also confusing. So, a class/object in DSL will have whatever methods they really have, plus any "add-on methods" provided by extensions. But, my point is/was that calling Item.sendCommand(String) and sendCommand(Item, String) ends up calling the exact same method, the result should be exactly the same.

It used to be the case that you couldn't send a DecimalType as a command to a Number Item because the Number Item class had both a sendCommand(DecimalType) and a sendCommand(Number) and because DecimalType is a Number Rules DSL would through an "am,biguous method call" exception.

That's a bit strange, but I understand the dilemma. DecimalType is probably a Command, but I'm a bit confused. If Item.sendCommand(DecimalType) existed (I don't think it does), it should be picked by specificity. Instead, I think what you really have is Item.sendCommand(Command). Since a DecimalType implements both Command and Number, they might be of "equal specificity" and result in ambiguity. It should be fairly easy to solve though, because instead of these two methods, there could be only Item.sendCommand(Object). That would allow the code inside that method to decide how to deal with different types - obviously, Command would have preference over Number. The method could handle String conversion etc. as well, and then throw an exception if it received a type it can't do anything sensible with.

In truth, the Item Object you see actually in the rule has tons more than the Item class would imply.

Yes, because of the extensions. I tried to say when I did the "helper methods" that this is somewhat of a mess, because every "action" is registered both as a method and as an extension. That doesn't make sense to me, and means that not only Item has "extra methods", but e.g. String and ZonedDateTime have quite a few non-standard methods added to them as well. I think it's high time to clean up some of this, but, as always, you never know what somebody has decided to use, no matter how crazy, so you'll probably break some scripts/rules out there even if you only remove the most unreasonable variants.

You cannot only rely on the JavaDocs and Java classes to understand how things work in Rules DSL. It's always been one of the things I hate about it.

Yes, and I'm beginning to understand why that is. There have been made quite a few decisions in OH that I think contribute to the confusion. But, in the end, it all "translates" to Java, and understanding the extension mechanism bridges a lot of "the gap" between Java and Xtend.

[rkoshak]

So, the method is explicitly made so that it won't throw an exception, only log it and carry on. It's a strange thing to do IMO, because the rest of the script probably won't work when you just "skip a step". It sounds to me like it's this very behavior that is the reason why avoiding sendCommand(string, string) is recommended. This behavior could very easily be changed so that it actually causes a failure.

While I did not write the above, I was part of the discussion. I used to spend roughly half my time on the forum helping people deal with this error.,

If Item.sendCommand(DecimalType) existed (I don't think it does), it should be picked by specificity. Instead, I think what you really have is Item.sendCommand(Command).

The error specifically mentions both these methods exist and it's confused between the one that takes DecimalType and the one that takes a Number. I don't know if this error still occurs or not. The error usually looks something like this (the first log I could find on the forum):

Thermostat_EG_WZ_rechts_Soll.sendCommand(Tado_HEAT_target_temperature.state as DecimalType)
Ambiguous feature call.
The extension methods
sendCommand(Item, Command) in BusEvent and
sendCommand(Item, Number) in BusEvent
both match.

I've seen it with HSBType, I've seen it with DecimalType and a few others.

But that error is not really realted to this section of the docs.

Item.sendCommand(String) and sendCommand(Item, String) ends up calling the exact same method, the result should be exactly the same.

sendCommand(Item, String) I think is relatively new. The evet bus action used to only support sendCommand(String, String).

And this section was to tell users to use the sendCommand on the Item Object instead of calling the even bus action directly. That's the only reason it exists,.

[Nadahar]

While I did not write the above, I was part of the discussion. I used to spend roughly half my time on the forum helping people deal with this error.,

But don't you agree that the whole reason why this method is "problematic" is that it doesn't throw an error, it just fails to do what it should, logs a warning, and then returns as if as was well..? And this is made so very deliberately, so if a lot of time has been wasted on this problem, then there has to have been a lack of coordination between "different parts" of the team.

The error specifically mentions both these methods exist and it's confused between the one that takes DecimalType and the one that takes a Number. I don't know if this error still occurs or not. The error usually looks something like this (the first log I could find on the forum):

Thermostat_EG_WZ_rechts_Soll.sendCommand(Tado_HEAT_target_temperature.state as DecimalType)
Ambiguous feature call.
The extension methods
sendCommand(Item, Command) in BusEvent and
sendCommand(Item, Number) in BusEvent
both match.

The error you show confirms exactly what I was saying. There is no sendCommand(Item, DecimalType) (or it would have matched), but there are sendCommand(Item, Command) and sendCommand(Item, Number). If you look at the definition of DecimalType, you find this:

public class DecimalType extends Number implements PrimitiveType, State, Command, Comparable<DecimalType> {

So, it extends Number and implements Command. They end up "on the same level", they are both "parents" one level above DecimalType. That's why it's ambiguous, because there's no "rule" that says which one to pick when they are of "equal specificity".

The documentation states that there is a sendCommand(DecimalType) (or sendCommand(Item, DecimalType) if you will), and this is wrong and part of what confuses me when I read it - because if that was the case, it would have behaved differently. It's the same with sendCommand(int) etc., these methods don't exist, and if they did, the behavior would have been different that what it is (and what the documentation tries to explain). It's these contradictions that make it very hard to understand, for me at least. Maybe it's less confusing if you know less about how these things actually work "under the hood", because you can just kind of "accept it as a fact".

sendCommand(Item, String) I think is relatively new. The evet bus action used to only support sendCommand(String, String).

I'm afraid it's not, and this is something I criticized during #5481. The problem is the way the "ActionClasses" (everything provided by actionServiceProviders and thingActionsProviders) is registered. They are registered both as ExtensionClasses and StaticImportClasses. If you look at BusEvent, which isn't one of the "ActionClasses", but is still registered both places so it has the "same effect", you will see definitions like this:

https://github.com/openhab/openhab-core/blob/77fa7c4a33992ed32775df11663f2c37fccc44fe/bundles/org.openhab.core.model.script/src/org/openhab/core/model/script/actions/BusEvent.java#L30-L70

Registering them as both, means that each method is registered twice, in slightly different form. If we take the first one:

    public static Object sendCommand(Item item, String commandString) {
        BusEventActionService.getBusEvent().sendCommand(item, commandString);
        return null;
    }

When registering this as an extension, the first argument is "erased" and instead indicates the class which it should extend. So, the result is Item.sendCommand(String commandString). But, since the same method is also registered as a static import, it can also be invoked with sendCommand(Item item, String commandString) (that is, exactly as defined). Both will work, and both forms ends up calling the exact same method.

When looking at many of the methods, it's clear to me that many were never "intended" to be registered as both. Take this for example:

    /**
     * Schedules a block of code for later execution.
     *
     * @param identifier an optional identifier
     * @param zonedDateTime the point in time when the code should be executed
     * @param closure the code block to execute
     * @return a handle to the created timer, so that it can be canceled or rescheduled
     */
    Timer createTimer(@Nullable String identifier, ZonedDateTime zonedDateTime, Runnable closure);

IMO, this was never intended to be an extension. It will extend String itself. So, in DSL you can actually do "myTimerName".createTimer(ZonedDateTime, Runnable). This makes no sense to me, and I'm betting that most of these aren't documented anywhere and aren't known/used. This one should simply have never been registered as extension, it should only have been a static import. Other methods look more like they are only intended as extensions, but it's less "clear cut" that way - they can usually also be useful as static imports even if they make sense as extensions.

This has been around for quite some time, you can see some of it here: openhab/openhab-core#1594. So, there seems to have been some back and forth when it comes to registering them as extensions, but I think they have been in the static import form (sendCommand(Item, Command)) pretty much the whole time (digging through all the history just to figure out the specifics isn't that tempting).

While something of a "detour", my point is that sendCommand(Item, String) is most likely as old as, or older than, Item.sendCommand(String). And, the way the documention is written, there is no mention of sendCommand(String, String), which seems to be the "real enemy", what is described is the "static import form", which is called "Generic (Action)": sendCommand(MyItem, new_state). It is in fact just as much a method as the "other form", and not only that, it's actually the exact same method.

The evet bus action used to only support sendCommand(String, String).

Both forms are from the "event bus action". In fact, the "problematic method" is also registered as both:

    public static Object sendCommand(String itemName, String commandString) {
        BusEventActionService.getBusEvent().sendCommand(itemName, commandString);
        return null;
    }

..which means that you can invoke this "silently fail" method both using sendCommand("MyItem", "command") and using "MyItem".sendCommand("command"). Both these forms will call the same method, and fail equally silent, just logging a warning and then carrying on with the rule execution.

[rkoshak]

But don't you agree that the whole reason why this method is "problematic" is that it doesn't throw an error, it just fails to do what it should, logs a warning, and then returns as if as was well..?

Not really. Send an update or a command to an Item has never been a blocking action. Even when successfull, I cannot, for example, postUpdate to an Item and then within the next few lines of code use MyItem.state and get the same state I just posted. It will not have changed state yet.

I don't have a problem if it threw an exception but the current behavior doesn't bother me. It's been this way sonce OH 1.3 or before. (1.3 is the first version I used briefly).

And this is made so very deliberately, so if a lot of time has been wasted on this problem, then there has to have been a lack of coordination between "different parts" of the team.

Throwing an exception would not have changed the amount of time I spent on the forum helping people solve the problem. Whether the users get a warning or an exception, they still see something is wrong and I still had to help them figure out how to get past it. I will note that it has been quite some time since this has come up on the forum. I don't know if that's because most new users choose something other than Rules DSL or if something changed so the problem doesn't occur as often.

The documentation states that there is a sendCommand(DecimalType) (or sendCommand(Item, DecimalType) if you will), and this is wrong and part of what confuses me when I read it -

It was correct when that section was written.

The documentation states that there is a sendCommand(DecimalType) (or sendCommand(Item, DecimalType) if you will), and this is wrong and part of what confuses me when I read it - because if that was the case, it would have behaved differently. I

Prior to 2018 (I don't know when) the error used to say sendCommand(String DecimalType) and sendCommand(String, Number). The LSP autocomplete in VS Code also used to show these as methods. I'm sure this changed at some point and the docs were not updated.

But again, the whole point of the section is just to use the extension methods on the Item rather than using the event bus actions where possible.

think is relatively new. T

When I say relatively new, I meain since OH 2.5.

And the relatively new part is passing an Item as the first argument instead of the String. At some point there was a separate function that took a String and only that function existed as something that was docmented.

[Nadahar]

Not really. Send an update or a command to an Item has never been a blocking action. Even when successfull, I cannot, for example, postUpdate to an Item and then within the next few lines of code use MyItem.state and get the same state I just posted. It will not have changed state yet.

I'm not talking about blocking, but about accepting invalid types. Isn't the whole argument for using the extension version that this will make your script fail, while using the string version won't, so it's harder to "discover" that you use an invalid type? That is what throwing an exception there would change, it would make the "string version" behave like the extension version.

When it comes to how used to be back a long time ago, I don't know, and I don't think it's worth the time to figure it out. The fact is still that, as this is documented, it isn't correct today.

[rkoshak]

At this point I haven't used Rules DSL in years. I campaigned for it's removal at one point. And I only minimally support it on the forum. That's a long way to say I don't really care if you change these docs or not. All I can say is why they are there in the first place. If they are no longer correct they should be changed. But I'm not going to do it,.