RDKEMW-17968: irdb store manufac model info - #240
Conversation
There was a problem hiding this comment.
Pull request overview
Adds manufacturer/model metadata plumbing to the IRDB programming flow so ControlMgr can persist and surface the device’s make/model alongside the programmed IR code set IDs.
Changes:
- Extend IR programming request data to carry
manufacturerandmodelfrom IRDB lookup through to network programming. - Persist manufacturer/model for TV and AVR in both the IR RF DB storage and per-controller DB attributes, and expose them in BLE controller status/logging.
- Add database read/write helpers and load/store integration for the new fields.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/network/ctrlm_ir_rf_db.h | Extends add_irdb_codes API to accept manufacturer/model and adds new stored fields. |
| src/network/ctrlm_ir_rf_db.cpp | Stores/clears manufacturer/model with IR code IDs, prints them in debug, and loads/stores them to DB. |
| src/irdb/ctrlm_irdb_interface.h | Adds manufacturer/model to the main-queue IR programming message and updates private _program_ir_codes signature. |
| src/irdb/ctrlm_irdb_interface.cpp | Caches last manufacturer/model from entry-id lookup and forwards it into IR programming requests. |
| src/database/ctrlm_database.h | Declares DB helpers for reading/writing TV/AVR manufacturer/model. |
| src/database/ctrlm_database.cpp | Implements DB helpers and adds new keys for manufacturer/model persistence. |
| src/ctrlm_controller.h | Adds controller API + DB-backed attributes to store IRDB manufacturer/model per device type. |
| src/ctrlm_controller.cpp | Implements manufacturer/model persistence and loads/stores the new attributes. |
| src/ble/ctrlm_ble_network.cpp | Passes manufacturer/model into IR RF DB programming and persists it on the controller object. |
| src/ble/ctrlm_ble_controller.cpp | Prints stored IRDB manufacturer/model in BLE controller status output. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/rf4ce/ctrlm_rf4ce_network.cpp:4263
- RF4CE IR programming path isn't propagating the newly added manufacturer/model (and also doesn't pass vendor info) into the IR RF DB or the controller attributes. As a result, RF4CE controllers won't persist manufacturer/model and the IR RF DB may store vendor info as defaults (0/empty). Update this to use
dqm->manufacturer,dqm->model, anddqm->vendor_infolike the BLE path does, and persist manufacturer/model on the controller.
XLOGD_INFO("Setting IR Codes on Controller %u", dqm->controller_id);
unsigned char status[1] = {IR_RF_DATABASE_STATUS_DB_DOWNLOAD_YES | IR_RF_DATABASE_STATUS_FORCE_DOWNLOAD};
ir_rf_database_.add_irdb_codes(dqm->ir_codes);
XLOGD_INFO("\n%s", this->ir_rf_database_.to_string(true).c_str());
controllers_[dqm->controller_id]->rf4ce_rib_set_target(CTRLM_RF4CE_RIB_ATTR_ID_IR_RF_DATABASE_STATUS, CTRLM_RF4CE_RIB_ATTR_INDEX_GENERAL, CTRLM_RF4CE_RIB_ATTR_LEN_IR_RF_DATABASE_STATUS, status);
controllers_[dqm->controller_id]->irdb_entry_id_name_set(CTRLM_IRDB_DEV_TYPE_TV, ir_rf_database_.get_tv_ir_code_id());
controllers_[dqm->controller_id]->irdb_entry_id_name_set(CTRLM_IRDB_DEV_TYPE_AVR, ir_rf_database_.get_avr_ir_code_id());
this->ir_rf_database_.store_db();
| if (this->value.empty()) { | ||
| XLOGD_DEBUG("value is empty for db key <%s>", this->get_name().c_str()); | ||
| } else if(blob.from_string(this->value)) { |
| void ctrlm_db_tv_manufacturer_write(const std::string manufacturer); | ||
| void ctrlm_db_tv_manufacturer_read(std::string &manufacturer); | ||
| void ctrlm_db_tv_model_write(const std::string model); | ||
| void ctrlm_db_tv_model_read(std::string &model); | ||
| void ctrlm_db_avr_manufacturer_write(const std::string manufacturer); | ||
| void ctrlm_db_avr_manufacturer_read(std::string &manufacturer); | ||
| void ctrlm_db_avr_model_write(const std::string model); | ||
| void ctrlm_db_avr_model_read(std::string &model); |
…ing to avoid db write/read errors
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/ctrlm_controller.cpp:240
- Same persistence issue as TV: empty manufacturer/model won’t be stored in the DB, so clearing/unknown values may leave stale values behind across restarts. Normalize empty to a sentinel before writing.
if (irdb_manufacturer_avr_->to_string() != manufacturer) {
irdb_manufacturer_avr_->set_value(manufacturer);
ctrlm_db_attr_write(irdb_manufacturer_avr_);
}
if (irdb_model_avr_->to_string() != model) {
|
|
||
| if(!is_managed_by_network(dqm->controller_id)) { | ||
| XLOGD_ERROR("controller %d is not managed by the %s network", dqm->controller_id, name_get()); | ||
| XLOGD_WARN("controller %d is not managed by the %s network", dqm->controller_id, name_get()); |
| case CTRLM_IRDB_DEV_TYPE_TV: | ||
| if (irdb_manufacturer_tv_->to_string() != manufacturer) { | ||
| irdb_manufacturer_tv_->set_value(manufacturer); | ||
| ctrlm_db_attr_write(irdb_manufacturer_tv_); | ||
| } | ||
| if (irdb_model_tv_->to_string() != model) { | ||
| irdb_model_tv_->set_value(model); | ||
| ctrlm_db_attr_write(irdb_model_tv_); | ||
| } | ||
| break; |
… commit is unbuilt and untested
There was a problem hiding this comment.
🟡 Changes recommended
The RF4CE programming path and “empty manufacturer/model” handling currently risk dropping or failing to overwrite manufacturer/model metadata, leading to inconsistent/stale persisted values across networks.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/rf4ce/ctrlm_rf4ce_network.cpp:4253
- RF4CE IR programming drops manufacturer/model (and vendor) details that are now carried in the queue message, so RF4CE controllers/IRRF DB won’t persist the new info while BLE does. Pass the fields into add_irdb_codes() and persist them on the controller (mirroring the BLE path).
XLOGD_WARN("controller %d is not managed by the %s network", dqm->controller_id, name_get());
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Lite
| std::string manufacturer; | ||
| std::string model; | ||
|
|
||
|
|
||
| XLOGD_INFO("Programming IR codes for (%u, %u) with database id <%s>", network_id, controller_id, id.c_str()); | ||
| auto last_entry_itr = m_irdb_cache.find(id); |
|
b'## WARNING: A Blackduck scan failure has been waived A prior failure has been upvoted
|
No description provided.