Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix manufacturer specific cluster attributes with same ID as default attributes not being displayed/populated correctly #2

Merged
merged 5 commits into from
Aug 5, 2024
Merged

Fix manufacturer specific cluster attributes with same ID as default attributes not being displayed/populated correctly #2

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
857f6a1
Correct attribute handling with identical IDs
SwoopX Jul 26, 2024
24c1d75
Add missed code to display attribute details on doubleclick
SwoopX Jul 26, 2024
9c89628
Enhance debug output
SwoopX Jul 26, 2024
314bf4c
Fix SIGSEGV discovered on OTAU cluster
SwoopX Jul 26, 2024
e44c29a
Address PR comments
SwoopX Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/zm_attribute_info.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,7 @@ void zmAttributeInfo::readAttributeResponse(const deCONZ::ZclFrame &zclFrame)
{
//m_attribute.setDataType(dataType);
m_attribute.readFromStream(stream);
m_attribute.setManufacturerCode(zclFrame.manufacturerCode());
updateEdit();
ui->status->setText(tr("reading done"));
}
Expand Down
43 changes: 28 additions & 15 deletions src/zm_cluster_info.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ zmClusterInfo::zmClusterInfo(QWidget *parent) :
attrHeaders.append("type");
attrHeaders.append("access");
attrHeaders.append("value");
attrHeaders.append("mfc");
SwoopX marked this conversation as resolved.
Show resolved Hide resolved

m_attrModel->setHorizontalHeaderLabels(attrHeaders);

Expand Down Expand Up @@ -432,9 +433,14 @@ void zmClusterInfo::attributeDoubleClicked(const QModelIndex &index)
const deCONZ::ZclAttribute &attr = cluster->attributes()[i];
if (attr.id() == id)
{
info->setAttribute(m_endpoint, m_clusterId, m_clusterSide, attr);
ok = true;
break;
QString mfc = "0x" + QString("%1").arg(attr.manufacturerCode(), 4, 16, QLatin1Char('0')).toUpper();

if (mfc == m_attrModel->data(m_attrModel->index(index.row(), 5)).toString())
SwoopX marked this conversation as resolved.
Show resolved Hide resolved
{
info->setAttribute(m_endpoint, m_clusterId, m_clusterSide, attr);
ok = true;
break;
}
}
}

Expand Down Expand Up @@ -743,7 +749,7 @@ void zmClusterInfo::showAttributes()
{
m_attrModel->setRowCount(0);
//m_attrModel->setRowCount(m_cluster.attributes().size() + m_cluster.attributeSets().size());
m_attrModel->setColumnCount(5);
m_attrModel->setColumnCount(6);
ui->attrTableView->horizontalHeader()->stretchLastSection();
}

Expand Down Expand Up @@ -793,7 +799,7 @@ void zmClusterInfo::showAttributes()
m_attrModel->item(row, 1)->setBackground(palette().dark().color().lighter(120));
m_attrModel->item(row, 1)->setForeground(palette().brightText());

ui->attrTableView->setSpan(row, 1 , 1, 4);
ui->attrTableView->setSpan(row, 1 , 1, 5);

row++;
}
Expand Down Expand Up @@ -839,6 +845,9 @@ void zmClusterInfo::showAttributes()

data = attr.toString(dataType, deCONZ::ZclAttribute::Prefix);
m_attrModel->setData(m_attrModel->index(row, 4), data);

QString mfc = "0x" + QString("%1").arg(attr.manufacturerCode(), 4, 16, QLatin1Char('0')).toUpper();
m_attrModel->setData(m_attrModel->index(row, 5), mfc);

// visual difference if a attribute is available
for (int j = 0; j < m_attrModel->columnCount(); j++)
Expand All @@ -853,10 +862,8 @@ void zmClusterInfo::showAttributes()
}
else
{
for (uint i = 0; i < cluster->attributes().size(); i++)
for (const auto &attr : cluster->attributes())
{
const deCONZ::ZclAttribute &attr = cluster->attributes()[i];

if (discoveredOnly && !attr.isAvailable())
{
continue;
Expand All @@ -879,21 +886,26 @@ void zmClusterInfo::showAttributes()
m_attrModel->setRowCount(m_attrModel->rowCount() + 1);
}

m_attrModel->setData(m_attrModel->index(i, 0), aid);
m_attrModel->item(i, 0)->setData((uint)attr.id());
m_attrModel->setData(m_attrModel->index(row, 0), aid);
m_attrModel->item(row, 0)->setData((uint)attr.id());

m_attrModel->setData(m_attrModel->index(i, 1), attr.name());
m_attrModel->setData(m_attrModel->index(i, 2), dataType.shortname());
m_attrModel->setData(m_attrModel->index(i, 3), attr.isReadonly() ? "r" : "rw");
m_attrModel->setData(m_attrModel->index(row, 1), attr.name());
m_attrModel->setData(m_attrModel->index(row, 2), dataType.shortname());
m_attrModel->setData(m_attrModel->index(row, 3), attr.isReadonly() ? "r" : "rw");

data = attr.toString(dataType, deCONZ::ZclAttribute::Prefix);
m_attrModel->setData(m_attrModel->index(i, 4), data);
m_attrModel->setData(m_attrModel->index(row, 4), data);

QString mfc = "0x" + QString("%1").arg(attr.manufacturerCode(), 4, 16, QLatin1Char('0')).toUpper();
m_attrModel->setData(m_attrModel->index(row, 5), mfc);

// visual difference if a attribute is available
for (int j = 0; j < m_attrModel->columnCount(); j++)
{
m_attrModel->item(i, j)->setEnabled(attr.isAvailable());
m_attrModel->item(row, j)->setEnabled(attr.isAvailable());
}

row++;
}
}

Expand All @@ -904,6 +916,7 @@ void zmClusterInfo::showAttributes()
ui->attrTableView->resizeColumnToContents(2);
ui->attrTableView->resizeColumnToContents(3);
ui->attrTableView->resizeColumnToContents(4);
ui->attrTableView->setColumnHidden(5, true);
ui->attrTableView->resizeRowsToContents();
ui->attrTableView->horizontalHeader()->setStretchLastSection(true);
m_init = true;
Expand Down
18 changes: 6 additions & 12 deletions src/zm_controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6465,16 +6465,10 @@ void zmController::zclReadAttributesResponse(NodeInfo *node, const deCONZ::ApsDa

if (cluster)
{
for (uint i = 0; i < cluster->attributes().size(); i++)
for (auto &a : cluster->attributes())
{
auto &a = cluster->attributes()[i];
if (a.id() == id)
if (a.id() == id && a.manufacturerCode() == zclFrame.manufacturerCode())
SwoopX marked this conversation as resolved.
Show resolved Hide resolved
{
if (a.isManufacturerSpecific() && a.manufacturerCode() != zclFrame.manufacturerCode())
{
continue;
}

attr = &a;
break;
}
Expand All @@ -6495,9 +6489,9 @@ void zmController::zclReadAttributesResponse(NodeInfo *node, const deCONZ::ApsDa
if (dataType != attr->dataType())
{
DBG_Printf(DBG_ZCL, "ZCL Read Attributes node=0x%04X, error assumed data type "
" 0x%02X but got 0x%02X for at=0x%04X\n",
" 0x%02X but got 0x%02X for at=0x%04X, mfc=0x%04X\n",
node->data->address().nwk(),
attr->dataType(), dataType, attr->id());
attr->dataType(), dataType, attr->id(), attr->manufacturerCode());

// disabled by stack
if (dataType == deCONZ::ZclNoData)
Expand Down Expand Up @@ -6561,10 +6555,10 @@ void zmController::zclReadAttributesResponse(NodeInfo *node, const deCONZ::ApsDa
attr->setAvailable(false); // disable fetching
}

DBG_Printf(DBG_ZCL, "ZCL got data for node=0x%04X, cl=0x%04X, at=0x%04X, status=0x%02X, type=0x%02X\n",
DBG_Printf(DBG_ZCL, "ZCL got data for node=0x%04X, cl=0x%04X, at=0x%04X, mfc=0x%04X, status=0x%02X, type=0x%02X\n",
node->data->address().nwk(),
ind.clusterId(),
attr->id(), status, dataType);
attr->id(), attr->manufacturerCode(), status, dataType);
}

DBG_Assert(node && node->data && cluster);
Expand Down