[coap+tcp] Handling Signaling Option

[sbernard31]

Implementing a first draft version of #2092, I face some design issue to handle Signaling Option with current Californium Design.

5.2. Signaling Option Numbers

Option Numbers for Signaling messages are specific to the message
code. They do not share the number space with CoAP options for
request/response messages or with Signaling messages using other
codes.

See : CoAP Signaling Option Numbers Registry

Currently all Option design is based on the idea that "Option number" is enough to identify an option.
But Signaling Option are identified by "Signaling Code + Signaling Option number"

Note for #2092, I already add :

• new SignalingMessage which extends Message,
• new SignalingCode like Core or ResponseCode.

I create this issue to discuss what could be the right approach for SignalingOption ?

Here some ideas :

1. I feel this would be a kind of hack (I really don't like it) but maybe we could add use Option.number as a something which store Signaling Code + Signaling Option Number. (65000 to 65535) should not be used. (probably not to much code to change)
2. Creating a new SignalingOption class which inherit from Option with a new SignalingOptionNumberRegistry ? (more code but pretty much isolated)
3. Add Message code to Option API ? (which could be null)

Any preferences or other ideas ?

@boaks If there already known issue with current Option design ? Is there some plan about refactoring it ?
(I asked just in case a new design could solve known issue and this one)

[boaks]

CoAP over TCP should be renamed in "CoAP similar formated messages over a different protocol" ;-).

I plan to add some stuff to the option, but I didn't plan to add a second number domain ... I will see, if that will be possible.

By the way, my feeling is still, that these signaling messages are no "coap exchanges". That are mainly "signals" (one way messages) in coap format, even using different number domains.

[sbernard31]

CoAP over TCP should be renamed in "CoAP similar formated messages over a different protocol" ;-).

Yep, I feel like I've fallen down in a rabbit hole. 🙃

By the way, my feeling is still, that these signaling messages are no "coap exchanges". That are mainly "signals" (one way messages) in coap format, even using different number domains.

Yep I try to handle those signaling messages in Layer but without "coap exchanges".

void sendSignalingMessage(SignalingMessage message);
void receivedSignalingMessage(SignalingMessage message);

I plan to add some stuff to the option, but I didn't plan to add a second number domain ... I will see, if that will be possible.

Waiting I will try to explore the 2. way. (SignalingOption)

[sbernard31]

In csm branch you can have a look at Add SignalingOption commit.

I try to not change design too much but clearly this sounds not so satisfying.

After working on this, I think that maybe a better design (or at least a way to explore) for Option could be :

public class Option {
    OptionDefinition def;
    byte[] value;
}

public class OptionDefinition {
   int number;
   String name;
   OptionFormat format;
   int lengthMin;
   int lengthMax;
   byte[] default value;
   // ... ... 
   
   // and all needed methods like  :
   assertValueLength(int valueLength);
   /// ... ... 
}

public class/interface OptionNumberRegistry {
    // initialize with some value but I guess user could add its own definition.
   OptionDefinition getDefinition(int optionNumber)
}

public class/interface SignalingOptionNumberRegistry {
    OptionDefinition getDefinition(SignalingCode, int optionNumber)
}

OptionNumberRegistry and SignalingOptionNumberRegistry are mainly used by DataParser so it could be something dynamic not static, so you could populate registry differently for UDP and TCP.

[boaks]

The culprit is the OptionSet :-).

[sbernard31]

I have a bad feeling about OptionSet design but I think it should not be so affected by the design above.

Do you identify issue with OptionSet ?
Do you want I try create a branch (not related to TCP) to test this design ?

[boaks]

I'm unfortunately still busy with other tasks outside of Californium.
Not sure, if I can switch tomorrow my tasks, we will see.

[boaks]

I guess, OptionSet.addOption(Option) will cause pain.

[sbernard31]

I think it should be OK, you just need to give the optionDefinition when creating an Option.

For the code you point above ☝️, I did this change in cms branch :
In DataParser :

byte[] value = reader.readBytes(optionLength);
Option option = createOption(message, currentOptionNumber, value);
if (option != null) {
  optionSet.addOption(option);
}

And so TcpDataParser with new API will looks like :

public Option createOption(Message message, int optionNumber, byte[] value) {
  if (message instanceof SignalingMessage) {
          SignalingCode code = ((SignalingMessage)message).getCode();
          OptionDefinition def = SignalingOptionNumberRegistry.getDefinition(code, optionNumber)
	  if (def == null && OptionUtils.isCritical(optionNumber)) {
		  throw new IllegalArgumentException("Unknown critical option  " + optionNumber + "for" + code);
	  }
	 
	  return new Option(def, value);
  } else {
	  return super.createOption(message, optionNumber, value);
  }
}

[boaks]

I had a first view on the "Option" extension.
My alternative idea would be to use a specific (similar) "OptionNumberRegistry" for each option-number scope.
That requires to extend the OptionSet. It's just an idea and need more time to be evaluated, if it works.

[sbernard31]

I understand that you don't advice me to explore the Option way above ☝️ because you already plan to modify it ?

[boaks]

At least, I guess we will have two variants and then need to decide, which one will be chosen.

[sbernard31]

At least, I guess we will have two variants and then need to decide, which one will be chosen.

Currently, the OptionDefinition way is not written.

So still don't know if you encourage me to work on it OR if the design above is enough to decide which one will be chosen ?

And even harder question how do we make that decision ? 😬

[boaks]

That's more a question of the time lines.

If you need it now, then you need to do it. You may also just do it, in order to see, if the other changes are working. Then later, if an alternative gets available, sure, some of your work (or the work of the alternative) gets obsolete.

And even harder question how do we make that decision ?

As usual, if not agreed between the active committers, by "users voting".

[sbernard31]

That's more a question of the time lines.
If you need it now, then you need to do it.

Not really, I already have a short term solution.
The proposed design here is a mid/long term solution which could in my opinion enhance californium code base. (and by the way will allow to add signaling option in a more elegant way).

Then later, if an alternative gets available, sure, some of your work (or the work of the alternative) gets obsolete.

That doesn't sound efficient to me. I mean I will not implement it before to know if the current design sounds better to us.
It's OK to code it and drop it later if we find an issue we didn't see before.
But it's not OK if we already know that we disagree on the better design.

As usual, if not agreed between the active committers, by "users voting".

About users voting you already know my opinion about that.

Anyway, I think it's not a really good idea for me to try to provide PR about core californium feature.

[boaks]

In the case that someone wants to continue this work, feel free to comment or open an new issue.