Add RemoteControl (#88010)

[corneliusmunz]

Add support for the Remote Control (#88010) which is used in the train sets. I tried to use the poweredup device list CLI function but it hangs even after several tries and two different remote controls.

poweredup device list
Scan Started. Please select the Hub (using a number keys or 'q' to terminate):
1: RemoteControl (with address 180547298736503)
1
Selected RemoteControl with key 1
Discover Ports. Receiving Messages ...
...................................................................................................................

[j0nathan33]

+1, I receive same think with my remote control

[j0nathan33]

I enabled logging and It stop at this check :

info: SharpBrick.PoweredUp.Functions.DiscoverPorts[0]
Stage: 5/5, Messages: 110/115

[rickjansen-dev]

I debugged this a bit, and the device list command is not working/finishing because it expects response messages for everything but the responses to the PossibleModeCombinations port info request fail and respond with an error message: Error - InvalidUse from PortInformationRequest

This is the specific line that's causing trouble for this device:

powered-up/src/SharpBrick.PoweredUp/Functions/DiscoverPorts.cs

Line 71 in 9a3831e

await _protocol.SendMessageAsync(new PortInformationRequestMessage() { HubId = _hubId, PortId = port.PortId, InformationType = PortInformationType.PossibleModeCombinations, });

Apparently the call to get the possible mode combinations can fail and return an error message, maybe simply because this device doesn't support any mode combinations?
Maybe the error messages should be added to the KnowledgeManager.ApplyStaticProtocolKnowledge so they will be counted?

I removed the actual sending of the messages, nice and crude way to at least get the device list output for this device:
https://gist.github.com/vuurbeving/4addcbe8dc32c6bfc4bc295f616959c3

[tthiery]

Awesome work. I think the solution is indeed a warning but maybe also lowering the expected return messages when the request fails with a error message.

LWP is really a nice protocol, just when its prime implementer randomly acts weird ... it is annoying ;)

[tthiery]

btw.. since you have the hardware ... can you create the needed port dumps?

I also started a documentation project here: https://github.com/sharpbrick/docs ... not for the purpose of API documentation for this project, but as a dumping place for these port (mode) information dumps. They are flying around a little bit too easy ;)

[rickjansen-dev]

Sure, and since I had the port dumps, I also created the hub & devices. Couldn't resist, wanted to know if the buttons work 🙂. Created a draft PR.

#119

Some noteworthy stuff about this device:

• The RemoteControlButton has 5 modes, but only one of them (mode 4 - KEYSD) returns values. This is a multivalue mode and it returns the state of the three buttons as 3 seperate 0 or 1 values. I have no clue what the other modes are used for (the return no data)
• I changed the DevicePorts file to simply count generic error messages as received messages too, that will make the cli tool work and output all the port info. Not tested with other devices though, but as long as they don't return errors, it should work right? :)
• I commented HubProperty.SecondaryMacAddress in InitialHubPropertiesQueryAsync. The request for this property errors out. I'm not sure about the effects of this. This method might need to be reworked since there was also a line removed for the Technic Hub, maybe this list is quite a bit more Hub-specific? (I don't have the mario, but it seems related? Add Mario (from set 71360) #91 (comment))
• I have yet to test the RemoteControlRssi device, not sure if it works at all

[tthiery]

Regards the Properties
I will do something about it. This can be handled similar to the expected devices on the hub classes. I will create a PR for this for you to review (and merge into here)

Regards the Buttons
Cannot you also turn the controls on the device. Maybe the remaining ports are about the rotation (KEYSD - Keys Data): KEYR (Key Rotation). Have you sneaked peek into the other implementations? I/we will add this to the docs as open issues.

[rickjansen-dev]

You can turn the buttons yes, but there's no rotary encoder or whatever, sadly. That would've made it a great device to control lots of stuff. You can see the internals here: https://www.eurobricks.com/forum/index.php?/forums/topic/162288-powered-up-a-tear-down/

So I still have no idea where all the modes are for, since the button state of all three buttons can be retrieved with the KEYSD mode.

[rickjansen-dev]

So I only just figured out that a port can only be operated in a single (input?) mode at a time, which makes sense of course, I just never interpreted it that way for some reason 🤷‍♂️🤦‍♂️.

About the modes:

RCKEY mode: values , 0 for release, + = 1, red = 127, - = -1, if you press + and - at the same time you get 1 and then -1, if you release one of the buttons, you get 0. releasing the second button, no value/event
KEYA mode: seems to behave same as RCKEY
KEYR mode: seems to behave same as RCKEY
KEYD mode: bitmask mode kind of: + = 1, red = 2, - = 4 - output value is combined, for example pressing all 3 buttons at the same time results in value: 7
KEYSD mode: 3 seperate values are returned with button state (0,1) per button

So modes KEYD and KEYSD are similar in function but return the data in a different format. KEYD uses 1 byte, KEYSD uses 3 bytes for the same information. KEY(S)D seem to be the most useful mode to use, since with the other modes, you miss information if you press multiple buttons at once (when releasing only though).

RCKEY, KEYA & KEYR modes all seem to behave exactly the same. Maybe the difference is only in the way they register button press/depress, maybe some trigger on rising/falling edge, maybe some trigger on level, I'm not sure. Also I only tried things with delta interval 1.
Also in RCKEY, KEYA and KEYR mode, there's an issue with the red stop button, with the SI value mapping. The device reports the mode to have min and max values of -1 and 1. But the red button outputs 127 in these modes. This is mapped to the min/max scale and the resulting SI value is 12700, which does not fit in the sbyte.

powered-up/src/SharpBrick.PoweredUp/Protocol/Formatter/PortValueSingleEncoder.cs

Line 82 in 1f6198a

var pctValues = rawValues.Select(rv => Scale(rv, modeInfo.RawMin, modeInfo.RawMax, modeInfo.PctMin, modeInfo.PctMax)).Select(f => Convert.ToSByte(f)).ToArray();

So I guess Lego's interpretation of specifying min/max values is a bit wonky. Isn't 127 the special stop command for the (train) motor? Seems like an ugly hack to me.

---

If you ask me, I'd say let's only implement KEYD mode now, seperate into 3 booleans, one for each button and ignore the other modes for now. Document the way they work and if some other devices pop up that supplies input values outside of their own specified range, deal with it properly then? It does already throw a decent enough error, so it won't be hard to trace.
What are your thoughts on this @tthiery ?

[tthiery]

Agree with going ahead on your plan.

My educated guess for all these variants: Provide easy to convert stimulus for the hubs which pair with them. In the end they flow data from one port to another and someone has to convert it.

Next lesson learnt for you: min/max is a brain bug. Raw min/max is a scaling basis for the Si min/max or Pct min/max. In both ranges, raw and SI you can get values outside of the ranges. Read this about it.

[rickjansen-dev]

Hmm ok, so if I understand this correctly, raw min/max simply maps to si min/max or pct min/max, but raw values can exceed the raw range, and thus will be mapped to values outside the si or pct range too.

At first I didn't understand why you'd need that specific formula (I thought it was simply determining the factor between raw & si / pct ranges and multiplying that with the value) but seeing that example of a device that reports both raw and si ranges of 10 / 10 I can see why.

Now for this remote (in RCKEY, KEYA, KEYR modes), the formula does not work, or rather, it does work, but the type conversion afterwards fails :) The raw value range is [-1 .. 1] and the pct range is [-100 .. 100]. Now the red button outputs 127. This is translated to 12700 for the pct range, which is correct (weird though, but whatever). But this is then converted to the value type (in this case sbyte). This obviously does not work and throws an exception.
It is false to assume the mode data type is large enough to hold the mapped/scaled values. Not sure what's the best way to deal with this though, but I'd say we need a change for the SI & PCT datatypes to twice the size of the original (so for sbyte -> short, short -> int, int -> long, not sure about float though) I think this would hold the max factor * max value (for sbyte would be 127*127, which fits fine)

It's not an issue right now for this device, since I only need to use the KEYD mode, which does not have this issue. But I think we should create a separate issue to address this.

[tthiery]

Yeah .. I thought about that for some milliseonds when I implemented it and than was so happy that each conversion just worked that I forgot about it. Nasty. So far I do not see really a problem for practical use (PCT here is pretty useless considering that this is either 0 or 100... i suspect that they leave these default values in just to not emit 0 and 0), but I will keep an eye on it. Yeah, please separate it out.

[tthiery]

ps: Sorry for the late reply.