Alvaro Neira Ayuso wrote:
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c @@ -120,6 +120,12 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) case SBTS2050_TEMP_RQT: num = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + num = sizeof(command->cmd.pwrSetState); + break;
A small style remark is that I like to use: sizeof command->cmd.pwerSetState since sizeof isn't really a function. Similar to return.
@@ -138,6 +144,17 @@ static struct msgb *sbts2050_ucinfo_get(struct uc *ucontrol, struct ucinfo info) command->u8Id = info.id; command->u8Len = sizeof(command->cmd.tempGet); break; + case SBTS2050_PWR_RQT: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrSetState); + command->cmd.pwrSetState.u1MasterEn = !!info.master; + command->cmd.pwrSetState.u1SlaveEn = !!info.slave; + command->cmd.pwrSetState.u1PwrAmpEn = !!info.pa; + break; + case SBTS2050_PWR_STATUS: + command->u8Id = info.id; + command->u8Len = sizeof(command->cmd.pwrGetStatus);
The above lines have a couple of extra spaces before the = which may or may not be worth fixing.
+int sbts2050_uc_status(struct uc *ucontrol, enum sbts2050_status_rqt status) .. + msg = sbts2050_ucinfo_get(ucontrol, info); + + if (msg == NULL) { + LOGP(DTEMP, LOGL_ERROR, "Error switching off some unit");
This error message doesn't contain very much information. It might be nice to know both which unit could not be switched off *and* what the error was. Also, is DTEMP guaranteed to always be the appropriate subsystem for this function? Maybe yes, but this code is in sysmobts_misc.c which suggests that it may be something more general?
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.h @@ -13,6 +13,12 @@ enum sysmobts_temp_type { _NUM_TEMP_TYPES };
+enum sbts2050_status_rqt { + SBTS2050_STATUS_MASTER, + SBTS2050_STATUS_SLAVE, + SBTS2050_STATUS_PA
Maybe add a , after the last enum to make it easier to add more values in the future. //Peter