This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.
Álvaro Neira Ayuso alvaroneay at gmail.comHello Peter El 15/03/14 02:45, Peter Stuge escribió: > 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. I have followed the kernel coding style, and he said doesn't speak about use sizeof without bracket. > > >> @@ -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. Yes, it's true, I don't know why I have done that... > > >> +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? I have used DTEMP, because i'm going to have a relation between the temperature and this function but it's true that somebody can use it in other cases. I don't know what do you think about that, if somebody can help me. > > >> +++ 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 > Thanks a lot Peter