arduino-dsmr icon indicating copy to clipboard operation
arduino-dsmr copied to clipboard

Allow customizing mbus id allocations

Open mrWheel opened this issue 4 years ago • 9 comments

Is it not a good idea to rename the gas_, thermal_ and water_ fields to mbus1_, mbus2_ and mbus3_ respectively?

People can than dynamically name these fields in there programs depending on the device connected.

I think it would solve lots of the confusion as it would not assume the type of device connected.

Maybe you could even supply a function to “connect” for instance “gas” to “mbus3” and so on ...

mrWheel avatar Mar 07 '20 11:03 mrWheel

Good thoughts. This is indeed a flaw in the current library, that it makes too much assumptions.

Another option might be to pass the mbus id to all fields that need it? E.g. something like:

constexpr int gas_mbus_id = 1;
using MyData = ParsedData<
  /* TimestampedFixedValue */ gas_delivered<gas_mbus_id>,
>;

OTOH, that might cause a lot of verbosity, and might suggest that you could have multiple e.g. gas_delivered values in a single datagram (but that does not actually work, since they would both have the same name).

Looking more at your proposal: I do not think that renaming e.g. gas to mbus1 would really help, since that loses meaning on the fields (the fields are gas-meter specific. What we could do is duplicate the fields (define mbus1_gas_delivered, mbus2_gas_delivered, etc, but that needs more duplication that I'd like.

Maybe the best solution would be to have a global gas_mbus_id variable (replacing the current GAS_MBUS_ID constant) that defaults to 1 and can be changed from the sketch. This does require some additional machinery to make sure that the value is read from the variable at runtime, rather than at compiletime.

matthijskooijman avatar Mar 07 '20 13:03 matthijskooijman

The real question is: “are you going to invest some time in this and update the library?”. I really would like to help but the whole template implementation is way over my head so the changes have to come from you.... There are now more then 400 DSMR-logger (like the one I send to you) out there that use my software with your great library but it desperately need this update and some mechanisme to use this library with pré 4.0 Slimme Meters and adjustment for the Belgium market (I now have forked your library two times to provide for this, but thats not really the way I want to go).

mrWheel avatar Mar 07 '20 15:03 mrWheel

The real question is: “are you going to invest some time in this and update the library?”.

It's on my "Soon" list on my personal Trello to do some maintainance and development on DSMR again. There's two other things eating up my time now, but I should get around to this in near future.

I really would like to help but the whole template implementation is way over my head so the changes have to come from you....

It would be valuable if you could help figure out how it should best look in the sketch code, then I'll see how to implement things in template land :-)

matthijskooijman avatar Mar 08 '20 11:03 matthijskooijman

It's on my "Soon" list on my personal Trello to do some maintainance and development on DSMR again. There's two other things eating up my time now, but I should get around to this in near future.

Can you elaborate on what your near future is? ;-)

It would be valuable if you could help figure out how it should best look in the sketch code, then I'll see how to implement things in template land :-)

From my point of view I would very much like this:

  1. a method to dynamically select a range of DSMR standards. At this point in time these are
    • the default (DSMR 4.0 and higher - as long as they are compatible)
    • pré 4.0 (no checksum)
    • Belgium standard (seems to be 5.0.2)

Something like setStandard(PRE40) or DUTCH40 or BELGIUM50 as pre defined types.

  1. a method to dynamically set the MBUS 1,2,3 and 4 to a specific type of device. Something like “setMbus(WATER_TYPE, GAS_TYPE, THERMO_TYPE, SLAVE_TYPE) where the X_TYPEs are somewhere pre defined and the position sets the MBUS.

If you can make this happen I can upgrade my firmware with dynamically setting so the “maker” does not have to compile different versions for different Slimme Meters. Just one binary and during startup it will read the settings and behave the way you want! Can’t wait!

mrWheel avatar Mar 08 '20 17:03 mrWheel

Maybe you can use a struct like this:

  typedef struct {
      uint8_t   protocol_version;  // 0 = Dutch (default), 1 = Belgium, 2 = pre40 etc.
      uint8_t   mbus1_type;        // 0 = gas (default), 1 = water, 2 = thermal, 3 = slave
      uint8_t   mbus2_type;        // 1 = water (default), 2 = thermal, 3 = slave, 0 = gas
      uint8_t   mbus3_type;        // 2 = thermal (default), 3 = slave, 0 = gas, 1 = water
      uint8_t   mbus4_type;        // 3 = slave (default), 0 = gas, 1 = water, 2 = thermal
  } dsmrConfig;

and than optional add the dsmrConfig to the parser

    slimmeMeter.parse(&DSMRdata, &DSMRerror, dsmrConfig)   // use user defined settings

or

    slimmeMeter.parse(&DSMRdata, &DSMRerror)               // use default settings

It would certainly give me all the options I need!

mrWheel avatar Mar 10 '20 10:03 mrWheel

I thought about how I could implement this a bit more, and came up with the following approach. This is partly a request for comments, but also meant to not forget my thoughts :-)

  1. The existing code, where you just use e.g. gas_delivered directly in the ParsedData type, will work as before (with a hardcoded, unchangeable mbus id).

  2. You will be able to assign a custom, compiletime constant mbus id to some fields by wrapping them in a new custom type. This would look something like this:

    using MyData = ParsedData<
      /* String */ identification,
      gas </* mbus_id */ 1,
          /* TimestampedFixedValue */ gas_delivered<gas_mbus_id>,
          /* String */ gas_equipment_id,
      >,
    >;
    

    This wraps the gas-specific fields in a new gas type, which gets the mbus id passed at compiletime. Underwater, I think I can implement this by letting the parsing function on the gas type forward to each of the contained fields (similar to how ParsedData does this), and then passing the mbus id to each field's parse function. These parse functions would then have this extra mbus id parameter, but with a default value so directly using these fields (as under 1.) will keep working.

    To access the data, you would do something like this:

    MyData data;
    ...
    Serial.println(data.gas.gas_delivered);
    
  3. When you want to configure the mbus id at runtime, rather then compiletime, you can just omit it (I'll have to see if I can figure this out template-wise, but I think I can provide specializations based on the type of the first template argument). E.g. you would leave out the mbus id:

     using MyData = ParsedData<
      /* String */ identification,
      gas <
          /* TimestampedFixedValue */ gas_delivered<gas_mbus_id>,
          /* String */ gas_equipment_id,
      >,
    >;
    

    And then configure it at runtime:

    MyData data;
    data.gas.mbus_id = 1;
    
  4. You might also need to have two gas meters attached. This could be supported by defining a custom MBUS slave type. e.g.

     DEFINE_MBUS_SLAVE(my_gas);
     DEFINE_MBUS_SLAVE(my_other_gas);
    
     using MyData = ParsedData<
      /* String */ identification,
      my_gas </* mbus_id */ 1,
          /* TimestampedFixedValue */ gas_delivered<gas_mbus_id>,
          /* String */ gas_equipment_id,
      >,
      my_other_gas </* mbus_id */ 2,
          /* TimestampedFixedValue */ gas_delivered<gas_mbus_id>,
          /* String */ gas_equipment_id,
      >,
    >;
    

    These custom mbus slave types can be used just like the builtin gas type. The DEFINE_MBUS_SLAVE macro might need to be passed a default mbus id (to be used when the id is not specified in the macro), though maybe that is not really needed (and also not for the builtin gas type either?), or it could just default to 0 or 1. That could prevent having to specify the value twice when using a compiletime fixed value.

    Maybe we do not even need any builtin gas type, and can just let the sketch always call DEFINE_MBUS_SLAVE if it needs to wrap the mbus slave fields in a type?

    I'm not entirely sure I like the syntax of specifying the mbus id template parameter to these types either, so that could maybe also be resolved by moving any compiletime constant mbus_id to the DEFINE_MBUS_SLAVE macro and never specifying it inside the ParsedData block (and omitting the id from the DEFINE_MBUS_SLAVE macro would make it runtime-configurable. Thinking about this, this is probably better indeed.

  5. It might be needed to disable an mbus slave at runtime too, so maybe mbus_id 0 (or 255) could be used for that (probably through a constant).

matthijskooijman avatar Mar 29 '20 12:03 matthijskooijman

@matthijskooijman I wish I could give some intellectual input on this .. but I can't!

For my usage option/idea 3 is great. In that scheme I can just code

     MyData data;
     data.gas.mbus_id = 1;

And after reading new settings I can code:

    data = {}
    data.gas.mbus_id = 2;

and from that moment on the gas_values are taken from mbus_id 2!! Fantastic! Does this change the field naming? Or will the gas naming still be: gas_device_type, gas_equipment_id, gas_valve_position and gas_delivered? (I hope so!)

mrWheel avatar Mar 29 '20 13:03 mrWheel

Changing the id would not change the field, but the fields would be named differently than now, e.g.:

Serial.println(data.gas.gas_delivered);

matthijskooijman avatar Mar 29 '20 14:03 matthijskooijman

Changing the id would not change the field, but the fields would be named differently than now, e.g.:

Serial.println(data.gas.gas_delivered);

Thats totally acceptable! Makes sense!

mrWheel avatar Mar 29 '20 14:03 mrWheel