Meter Identification example added #37961
Conversation
popovdg
force-pushed
the
meter_identification-example
branch
from
4d37dea to
b80b9d0
Compare
|
|
||
| static void SetCharSpan(DataModel::Nullable<CharSpan> & charSpan, const DataModel::Nullable<CharSpan> && value) | ||
| { | ||
| if (!value.IsNull()) |
There was a problem hiding this comment.
Shouldn't the set work even if null?
| const size_t len = value.Value().size(); | ||
| char * str = (char *) chip::Platform::MemoryAlloc(len + 1); | ||
| strncpy(str, value.Value().data(), len); | ||
| str[len] = 0; |
There was a problem hiding this comment.
Why? CharSpans are not null-terminated.
| if (!value.IsNull()) | ||
| { | ||
| const size_t len = value.Value().size(); | ||
| char * str = (char *) chip::Platform::MemoryAlloc(len + 1); |
There was a problem hiding this comment.
Needs to handle allocation failure?
| char * str = (char *) chip::Platform::MemoryAlloc(len + 1); | ||
| strncpy(str, value.Value().data(), len); | ||
| str[len] = 0; | ||
| charSpan = DataModel::MakeNullable(CharSpan::fromCharString(str)); |
There was a problem hiding this comment.
| charSpan = DataModel::MakeNullable(CharSpan::fromCharString(str)); | |
| charSpan = DataModel::MakeNullable(CharSpan(str, len)); |
| if (!value.IsNull()) | ||
| { | ||
| const size_t len = value.Value().size(); | ||
| char * str = (char *) chip::Platform::MemoryAlloc(len + 1); |
There was a problem hiding this comment.
| char * str = (char *) chip::Platform::MemoryAlloc(len + 1); | |
| auto * str = static_cast<char *>(chip::Platform::MemoryAlloc(len)); |
| { | ||
| if (!mPointOfDelivery.IsNull()) | ||
| { | ||
| chip::Platform::MemoryFree((void *) mPointOfDelivery.Value().data()); |
There was a problem hiding this comment.
Let's avoid hidden const_cast, please.
| return CHIP_NO_ERROR; | ||
| } | ||
|
|
||
| if(static_cast<std::underlying_type<MeterTypeEnum>::type>(MeterTypeEnum::kUnknownEnumValue) <= |
There was a problem hiding this comment.
You can't assume kUnknownEnumValue is larger than all valid enum values. The only thing you can do with kUnknownEnumValue is compare it for equality.
See also EnsureKnownEnumValue, which is the thing you may want here.
| return CHIP_NO_ERROR; | ||
| } | ||
|
|
||
| if ((newValue.Value().powerThreshold.HasValue() && INT64_MIN == newValue.Value().powerThreshold.Value()) || |
There was a problem hiding this comment.
Where is INT64_MIN coming from here? If this is meant exclude out-of-range values for a nullable, you should be using our existing mechanisms for checking for that. ExistingValueInEncodableRange on Nullable might do the right thing for you here?
| if ((newValue.Value().powerThreshold.HasValue() && INT64_MIN == newValue.Value().powerThreshold.Value()) || | ||
| (newValue.Value().apparentPowerThreshold.HasValue() && INT64_MIN == newValue.Value().apparentPowerThreshold.Value()) || | ||
| (!newValue.Value().powerThresholdSource.IsNull() && static_cast<std::underlying_type<PowerThresholdSourceEnum>::type>( | ||
| PowerThresholdSourceEnum::kUnknownEnumValue) <= static_cast<std::underlying_type<PowerThresholdSourceEnum>::type>( |
There was a problem hiding this comment.
See above about kUnknownEnumValue
|
|
||
| if (!dst.IsNull()) | ||
| { | ||
| chip::Platform::MemoryFree((void *) dst.Value().data()); |
There was a problem hiding this comment.
Strongly recommend not doing heap allocation here, but if we do see comments above about how it should probably work.
popovdg
force-pushed
the
meter_identification-example
branch
from
b80b9d0 to
4def522
Compare
Meter Identification cluster added to all-clusters-app example.