DeviceReplyMap: periodic member is uint8_t #87

New Issue

muellerr · 2020-05-28T12:50:18+02:00

muellerr commented

2020-05-28 12:50:18 +02:00

Is there a specific reason why periodic is a uint8_t instead of a boolean variable?

Same question also for PoolVariableIF valid member. Why not just make it boolean (which propably also makes the definitions VALID and INVALID obsolete) ? If uint8_t are required, a getter function for uint8_t could be used (even casting isnt necessary).

Is there a specific reason why periodic is a `uint8_t` instead of a `bool`ean variable? Same question also for `PoolVariableIF` valid member. Why not just make it boolean (which propably also makes the definitions VALID and INVALID obsolete) ? If uint8_t are required, a getter function for uint8_t could be used (even casting isnt necessary).

muellerr added the

question

label 2020-05-28 12:50:32 +02:00

muellerr changed title from ~~DeviceReplyMap: periodic member is `uint8_t`~~ to DeviceReplyMap: periodic member is uint8_t

2020-05-28 12:50:39 +02:00

muellerr referenced this issue

2020-06-18 14:54:10 +02:00

Change periodic parameter in DHB::DeviceReplyInfo to bool #111

mohr commented

2020-06-18 14:59:11 +02:00

As written in #111, I think DHB::DeviceReplyInfo::periodic would be better as bool.

As for the PoolVariableIF valid member, maybe in the future there will be different kinds of valid, so I would keep it for now.

As written in #111, I think `DHB::DeviceReplyInfo::periodic` would be better as `bool`. As for the `PoolVariableIF` valid member, maybe in the future there will be different kinds of valid, so I would keep it for now.

mohr added

feature

API Change

and removed

question

labels 2020-06-18 14:59:51 +02:00

mohr commented

2020-06-18 15:00:31 +02:00

While technically 1 and 0 as used currently can be cast to bool, I would suggest updating user code. Added API Change label.

muellerr commented

2020-06-19 00:43:36 +02:00

I think I integrated this already in the current state for the local data pools, but propably not in the current request.. But I think it would be a good idea to integrate the current request #44 first.