request for Mid code review NSoC 2011 - LTE RRC extension

[mmezzavilla, 2011-07-22 10:09:11]

Dear all,

my review is now available.



Best regards,

Marco

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity.cc
File src/lte/model/enb-rrc-cp-entity.cc (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.cc:58: EnbLteRrc::m_rrcStateInfo =
EnbLteRrc::RRC_IDLE;
m_rrcStateInfo = RRC_IDLE;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.cc:67: EnbLteRrc::RrcStates
EnbLteRrc::GetRrcState()
RrcStates EnbLteRrc::GetRrcState()

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity.h
File src/lte/model/enb-rrc-cp-entity.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:65: * To get the state of ue
* To get the state of UE

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:73: virtual void SendRrcIdealControlMessage
(Ptr<IdealControlMessage> msg);
virtual void SendRrcIdealControlMessage (Ptr<IdealControlMessage> msg) = 0;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:76: * Function to process rrc control signal
recieved at eNodeb
Function to process RRC control signal received at eNodeB

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:79: virtual void
ReceiveRrcIdealControlMessage(Ptr < IdealControlMessage > msg) ;
virtual void ReceiveRrcIdealControlMessage(Ptr < IdealControlMessage > msg) = 0;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:85: * State information
RRC State information

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:94: * \brief This class implements the RRC
control information regarding System Information , paging and call contrl
information
the \brief for this class is above, at the beginning of the class

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ideal-control-mess...
File src/lte/model/ideal-control-messages.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ideal-control-mess...
src/lte/model/ideal-control-messages.h:47: CQI_FEEDBACKS,
ALLOCATION_MAP,SYSTEM_INFO, PAGING, RRC_ESTABLISHMENT, RRC_CONNC_RECONFIG,
RRC_CONC_RELEASE
RRC_CONNC_RECONFIG and RRC_CONC_RELEASE refer to connection in two different
ways: CONNC and CONC, I would go for the same, for both. Maybe better is CONN.
It would be:
CQI_FEEDBACKS, ALLOCATION_MAP, SYSTEM_INFO, PAGING, RRC_ESTABLISHMENT,
RRC_CONN_RECONFIG, RRC_CONN_RELEASE

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
File src/lte/model/rrc-ideal-control-messages.cc (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:58:
reqmsg.m_idealRrcConnRequestMsg->m_causeEstablishment = cause;
ERROR: do not initiliaze an object of class RrcIdealConnectionreq inside the
function... you directly have a private member to work on.
You repeat this for all the 5 types of control messages, be careful. I simply
put  (*) from now on.
Then, you should have:
m_idealRrcConnRequestMsg->m_direction = direction;
m_idealRrcConnRequestMsg->m_signallingBearer = bearerid; 
m_idealRrcConnRequestMsg->m_rrcState = state;
m_idealRrcConnRequestMsg->m_causeEstablishment = cause;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:62:
RrcIdealConnectionreq::RrcConnectionReq*
RrcConnectionReq*

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:90:
msg.m_idealRrcRadioResourceConfig->m_reEstbCause = cause;
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:95:
RrcRadioResConfig::RadioResConfig*
RadioResConfig*

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:124:
rejectmsg.m_idealRrcConcRej->m_cause = cause;
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:129:
RrcConnectionReject::RrcConncRej*
RrcConncRej*

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:160:
simsg.m_systeminfo->m_q_RxLevMinOffset = RxLevMinOffset;
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:174:
mimsg.m_masterinfo->m_SystemFrameNumber = SystemFrameNumber;
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:179:
SystemInfoCtrlMesg::MasterInformationRecord*
MasterInformationRecord*

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:186:
SystemInfoCtrlMesg::SystemInformationRecord*
SystemInformationRecord*

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:210: msg.m_paginginfo->m_Imsi= IMSI;
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:215: Paging::PagingInfo*
PagingInfo*

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
File src/lte/model/rrc-ideal-control-messages.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:28: 
Among the objectives of the project, there is also the implementation of the
control message for resource configuration → (5.3.10, TS 36.331), but it looks
like here it isn't. Am I missing something? I am a bit confused because there is
a message defined as RRC_RADIO_RESOURCE_CONFIG_H, but then you do:
SetMessageType (IdealControlMessage::RRC_CONNC_RECONFIG). Is this referring to
the reconfiguration message (5.3.5), or to the resource configuration one
(5.3.10)? Or, are you merging them together?

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:31: * The RRC Connection
Establishment message between UE and ENodeB
I would sugget to insert a reference to the standard for all the control
messages, like this:

see section 5.3.3 of TS 36.331

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:66: /** the ue that receive  */
that receive...? (*) also below, for the other ctrl messages.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:69: int m_ueIdentity;
what kind of identifier?

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:135: RECONFIURATIONFAILURE,
RECONFIGURATIONFAILURE

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:144: /** the ue that receive  */
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:226: /** the ue that receive  */
(*)

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:227: Ptr<LteNetDevice> m_ue;
Is it not needed an integer m_ueIdentity now?

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:305: int m_q_RxLevMinOffset;
you may need to briefly explain why you set these variables m_q_RxLevMin an
m_q_RxLevMinOffset

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:379: int m_Imsi;
m_ueIdentity stands for S-TMSI and m_Imsi for IMSI or what? It is not so clear.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.cc
File src/lte/model/ue-rrc-cp-entity.cc (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.c...
src/lte/model/ue-rrc-cp-entity.cc:59: UeLteRrc::m_rrcStateInfo =
UeLteRrc::RRC_IDLE;
m_rrcStateInfo = RRC_IDLE;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.c...
src/lte/model/ue-rrc-cp-entity.cc:68: UeLteRrc::RrcStates
UeLteRrc::GetRRCState()
RrcStates UeLteRrc::GetRrcState()

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h
File src/lte/model/ue-rrc-cp-entity.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:58: * To get the state of ue
To get the state of UE

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:61: RrcStates GetRRCState();
RrcStates GetRrcState();

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:67: virtual void SendRrcIdealControlMessage
(Ptr<IdealControlMessage> msg);
virtual void SendRrcIdealControlMessage (Ptr<IdealControlMessage> msg) = 0;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:72: virtual void ReceiveRrcIdealControlMessage
(Ptr<IdealControlMessage> msg);
virtual void ReceiveRrcIdealControlMessage (Ptr<IdealControlMessage> msg) = 0;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:80: * Function to process rrc control signal
recieved at UE
RRC State information

[pankaj, 2011-08-01 18:31:24]

Dear all, 

I have updated the code as suggested by the reviewers . I have updated the
comments here 

http://codereview.appspot.com/4811043

I have initiated a new code review with the updated changes. 

http://codereview.appspot.com/4830049/ 

Please feel free to give your inputs. 


Thanks in advance 


Regards,
Pankaj

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity.cc
File src/lte/model/enb-rrc-cp-entity.cc (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.cc:58: EnbLteRrc::m_rrcStateInfo =
EnbLteRrc::RRC_IDLE;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> m_rrcStateInfo = RRC_IDLE;

modified as per the suggestion

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.cc:67: EnbLteRrc::RrcStates
EnbLteRrc::GetRrcState()
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RrcStates EnbLteRrc::GetRrcState()

no change . change gives compilation errors .

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity.h
File src/lte/model/enb-rrc-cp-entity.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:65: * To get the state of ue
On 2011/07/22 10:09:11, mmezzavilla wrote:
> * To get the state of UE


modified as per the suggestion

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:73: virtual void SendRrcIdealControlMessage
(Ptr<IdealControlMessage> msg);
On 2011/07/22 10:09:11, mmezzavilla wrote:
> virtual void SendRrcIdealControlMessage (Ptr<IdealControlMessage> msg) = 0;

It is made as a virtual function only  not a pure virtual function.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:76: * Function to process rrc control signal
recieved at eNodeb
On 2011/07/22 10:09:11, mmezzavilla wrote:
> Function to process RRC control signal received at eNodeB


modified as per suggetion.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:79: virtual void
ReceiveRrcIdealControlMessage(Ptr < IdealControlMessage > msg) ;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> virtual void ReceiveRrcIdealControlMessage(Ptr < IdealControlMessage > msg) =
0;

It is made as a virtual function only not a pure virtual function.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:85: * State information
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RRC State information

modified as per suggestion.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/enb-rrc-cp-entity....
src/lte/model/enb-rrc-cp-entity.h:94: * \brief This class implements the RRC
control information regarding System Information , paging and call contrl
information
On 2011/07/22 10:09:11, mmezzavilla wrote:
> the \brief for this class is above, at the beginning of the class

removed .

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ideal-control-mess...
File src/lte/model/ideal-control-messages.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ideal-control-mess...
src/lte/model/ideal-control-messages.h:47: CQI_FEEDBACKS,
ALLOCATION_MAP,SYSTEM_INFO, PAGING, RRC_ESTABLISHMENT, RRC_CONNC_RECONFIG,
RRC_CONC_RELEASE
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RRC_CONNC_RECONFIG and RRC_CONC_RELEASE refer to connection in two different
> ways: CONNC and CONC, I would go for the same, for both. Maybe better is CONN.
> It would be:
> CQI_FEEDBACKS, ALLOCATION_MAP, SYSTEM_INFO, PAGING, RRC_ESTABLISHMENT,
> RRC_CONN_RECONFIG, RRC_CONN_RELEASE

updated as per suggestion.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
File src/lte/model/rrc-ideal-control-messages.cc (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:58:
reqmsg.m_idealRrcConnRequestMsg->m_causeEstablishment = cause;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> ERROR: do not initiliaze an object of class RrcIdealConnectionreq inside the
> function... you directly have a private member to work on.
> You repeat this for all the 5 types of control messages, be careful. I simply
> put  (*) from now on.
> Then, you should have:
> m_idealRrcConnRequestMsg->m_direction = direction;
> m_idealRrcConnRequestMsg->m_signallingBearer = bearerid; 
> m_idealRrcConnRequestMsg->m_rrcState = state;
> m_idealRrcConnRequestMsg->m_causeEstablishment = cause;



updated with 

m_idealRrcConnRequestMsg->m_direction = direction;
m_idealRrcConnRequestMsg->m_signallingBearer = bearerid;
m_idealRrcConnRequestMsg->m_ueIdentity = uecontext;
m_idealRrcConnRequestMsg->m_rrcState = state;
	m_idealRrcConnRequestMsg->m_causeEstablishment = cause;

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:58:
reqmsg.m_idealRrcConnRequestMsg->m_causeEstablishment = cause;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> ERROR: do not initiliaze an object of class RrcIdealConnectionreq inside the
> function... you directly have a private member to work on.
> You repeat this for all the 5 types of control messages, be careful. I simply
> put  (*) from now on.
> Then, you should have:
> m_idealRrcConnRequestMsg->m_direction = direction;
> m_idealRrcConnRequestMsg->m_signallingBearer = bearerid; 
> m_idealRrcConnRequestMsg->m_rrcState = state;
> m_idealRrcConnRequestMsg->m_causeEstablishment = cause;

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:62:
RrcIdealConnectionreq::RrcConnectionReq*
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RrcConnectionReq*

change wil give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:90:
msg.m_idealRrcRadioResourceConfig->m_reEstbCause = cause;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

modified.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:90:
msg.m_idealRrcRadioResourceConfig->m_reEstbCause = cause;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:95:
RrcRadioResConfig::RadioResConfig*
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RadioResConfig*

change wil give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:124:
rejectmsg.m_idealRrcConcRej->m_cause = cause;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:129:
RrcConnectionReject::RrcConncRej*
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RrcConncRej*

change wil give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:160:
simsg.m_systeminfo->m_q_RxLevMinOffset = RxLevMinOffset;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:174:
mimsg.m_masterinfo->m_SystemFrameNumber = SystemFrameNumber;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:179:
SystemInfoCtrlMesg::MasterInformationRecord*
On 2011/07/22 10:09:11, mmezzavilla wrote:
> MasterInformationRecord*

change wil give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:186:
SystemInfoCtrlMesg::SystemInformationRecord*
On 2011/07/22 10:09:11, mmezzavilla wrote:
> SystemInformationRecord*

change wil give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:210: msg.m_paginginfo->m_Imsi= IMSI;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.cc:215: Paging::PagingInfo*
On 2011/07/22 10:09:11, mmezzavilla wrote:
> PagingInfo*

change wil give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
File src/lte/model/rrc-ideal-control-messages.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:28: 
On 2011/07/22 10:09:11, mmezzavilla wrote:
> Among the objectives of the project, there is also the implementation of the
> control message for resource configuration → (5.3.10, TS 36.331), but it looks
> like here it isn't. Am I missing something? I am a bit confused because there
is
> a message defined as RRC_RADIO_RESOURCE_CONFIG_H, but then you do:
> SetMessageType (IdealControlMessage::RRC_CONNC_RECONFIG). Is this referring to
> the reconfiguration message (5.3.5), or to the resource configuration one
> (5.3.10)? Or, are you merging them together?

updated , sorry for the confusion. it is for the reconfiguration section 5.3.5

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:31: * The RRC Connection
Establishment message between UE and ENodeB
On 2011/07/22 10:09:11, mmezzavilla wrote:
> I would sugget to insert a reference to the standard for all the control
> messages, like this:
> 
> see section 5.3.3 of TS 36.331

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:66: /** the ue that receive  */
On 2011/07/22 10:09:11, mmezzavilla wrote:
> that receive...? (*) also below, for the other ctrl messages.

removed.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:69: int m_ueIdentity;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> what kind of identifier?

it is taken as S-TMSI .

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:135: RECONFIURATIONFAILURE,
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RECONFIGURATIONFAILURE

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:144: /** the ue that receive  */
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:226: /** the ue that receive  */
On 2011/07/22 10:09:11, mmezzavilla wrote:
> (*)

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:227: Ptr<LteNetDevice> m_ue;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> Is it not needed an integer m_ueIdentity now? 


deleted m_ue and added m_ueIdenetity.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:305: int m_q_RxLevMinOffset;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> you may need to briefly explain why you set these variables m_q_RxLevMin an
> m_q_RxLevMinOffset

it is given in the specification just as added as part of SI information
element. May be utilized in later stages.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/rrc-ideal-control-...
src/lte/model/rrc-ideal-control-messages.h:379: int m_Imsi;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> m_ueIdentity stands for S-TMSI and m_Imsi for IMSI or what? It is not so
clear.

m_ueIdentity stands for s-TMSI ,  IMSI we are not including in our current
implementation.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.cc
File src/lte/model/ue-rrc-cp-entity.cc (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.c...
src/lte/model/ue-rrc-cp-entity.cc:59: UeLteRrc::m_rrcStateInfo =
UeLteRrc::RRC_IDLE;
On 2011/07/22 10:09:11, mmezzavilla wrote:
> m_rrcStateInfo = RRC_IDLE;

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.c...
src/lte/model/ue-rrc-cp-entity.cc:68: UeLteRrc::RrcStates
UeLteRrc::GetRRCState()
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RrcStates UeLteRrc::GetRrcState()


change will give compilation error.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h
File src/lte/model/ue-rrc-cp-entity.h (right):

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:58: * To get the state of ue
On 2011/07/22 10:09:11, mmezzavilla wrote:
> To get the state of UE

Done.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:61: RrcStates GetRRCState();
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RrcStates GetRrcState();

it is  similar.

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:67: virtual void SendRrcIdealControlMessage
(Ptr<IdealControlMessage> msg);
On 2011/07/22 10:09:11, mmezzavilla wrote:
> virtual void SendRrcIdealControlMessage (Ptr<IdealControlMessage> msg) = 0;

we have defined it as a virtual function but not as a pure virtual function

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:72: virtual void ReceiveRrcIdealControlMessage
(Ptr<IdealControlMessage> msg);
On 2011/07/22 10:09:11, mmezzavilla wrote:
> virtual void ReceiveRrcIdealControlMessage (Ptr<IdealControlMessage> msg) = 0;

we have defined it as a virtual function but not as a pure virtual function

http://codereview.appspot.com/4811043/diff/1/src/lte/model/ue-rrc-cp-entity.h...
src/lte/model/ue-rrc-cp-entity.h:80: * Function to process rrc control signal
recieved at UE
On 2011/07/22 10:09:11, mmezzavilla wrote:
> RRC State information

Done.

[pankaj, 2011-08-01 18:37:13]

Dear all,

I have updated the code as suggested by the reviewers . I have updated
the comments here


http://codereview.appspot.com/4811043

I have initiated a new code review with the updated changes.

http://codereview.appspot.com/4830049/

Please feel free to give your inputs.



Thanks in advance


Regards,
Pankaj