diff --git a/src/core/common/message.cpp b/src/core/common/message.cpp index 6a627257d..e2ff22982 100644 --- a/src/core/common/message.cpp +++ b/src/core/common/message.cpp @@ -328,31 +328,17 @@ bool Message::IsMleCommand(Mle::Command aMleCommand) const Error Message::SetPriority(Priority aPriority) { - Error error = kErrorNone; - uint8_t priority = static_cast(aPriority); - PriorityQueue *priorityQueue; + Error error = kErrorNone; + uint8_t priority = static_cast(aPriority); static_assert(kNumPriorities <= 4, "`Metadata::mPriority` as a 2-bit field cannot fit all `Priority` values"); VerifyOrExit(priority < kNumPriorities, error = kErrorInvalidArgs); - VerifyOrExit(IsInAQueue(), GetMetadata().mPriority = priority); - VerifyOrExit(GetMetadata().mPriority != priority); - - priorityQueue = GetPriorityQueue(); - - if (priorityQueue != nullptr) - { - priorityQueue->Dequeue(*this); - } + VerifyOrExit(!IsInAPriorityQueue(), error = kErrorInvalidState); GetMetadata().mPriority = priority; - if (priorityQueue != nullptr) - { - priorityQueue->Enqueue(*this); - } - exit: return error; } @@ -869,7 +855,7 @@ void Message::SetMessageQueue(MessageQueue *aMessageQueue) void Message::SetPriorityQueue(PriorityQueue *aPriorityQueue) { GetMetadata().mQueue = aPriorityQueue; - GetMetadata().mInPriorityQ = true; + GetMetadata().mInPriorityQ = aPriorityQueue != nullptr; } //--------------------------------------------------------------------------------------------------------------------- diff --git a/src/core/common/message.hpp b/src/core/common/message.hpp index f8cde13a8..17679ab9f 100644 --- a/src/core/common/message.hpp +++ b/src/core/common/message.hpp @@ -633,13 +633,15 @@ public: /** * Sets the messages priority. - * If the message is already queued in a priority queue, changing the priority ensures to - * update the message in the associated queue. + * + * If the message is already enqueued in a priority queue, the priority cannot be changed and `kErrorInvalidState` + * is returned. * * @param[in] aPriority The message priority level. * * @retval kErrorNone Successfully set the priority for the message. * @retval kErrorInvalidArgs Priority level is not invalid. + * @retval kErrorInvalidState Message is already queued in a priority queue. */ Error SetPriority(Priority aPriority); @@ -1569,6 +1571,8 @@ private: } bool IsInAQueue(void) const { return (GetMetadata().mQueue != nullptr); } + bool IsInAPriorityQueue(void) const { return GetMetadata().mInPriorityQ; } + void SetMessageQueue(MessageQueue *aMessageQueue); void SetPriorityQueue(PriorityQueue *aPriorityQueue); diff --git a/tests/unit/test_priority_queue.cpp b/tests/unit/test_priority_queue.cpp index 4642ee1e1..5cbf39334 100644 --- a/tests/unit/test_priority_queue.cpp +++ b/tests/unit/test_priority_queue.cpp @@ -135,44 +135,11 @@ void VerifyPriorityQueueContent(PriorityQueue &aPriorityQueue, int aExpectedLeng VerifyOrQuit(message == nullptr, "`for` loop iteration resulted in fewer entries than expected"); } -// This function verifies the content of the message queue to match the passed in messages -void VerifyMsgQueueContent(MessageQueue &aMessageQueue, int aExpectedLength, ...) -{ - va_list args; - Message *message; - Message *msgArg; - - va_start(args, aExpectedLength); - - if (aExpectedLength == 0) - { - message = aMessageQueue.GetHead(); - VerifyOrQuit(message == nullptr, "is not empty when expected len is zero."); - } - else - { - for (message = aMessageQueue.GetHead(); message != nullptr; message = message->GetNext()) - { - VerifyOrQuit(aExpectedLength != 0, "contains more entries than expected"); - - msgArg = va_arg(args, Message *); - VerifyOrQuit(msgArg == message, "content does not match what is expected."); - - aExpectedLength--; - } - - VerifyOrQuit(aExpectedLength == 0, "contains less entries than expected"); - } - - va_end(args); -} - void TestPriorityQueue(void) { Instance *instance; MessagePool *messagePool; PriorityQueue queue; - MessageQueue messageQueue; Message *msgNet[kNumTestMessages]; Message *msgHigh[kNumTestMessages]; Message *msgNor[kNumTestMessages]; @@ -264,7 +231,7 @@ void TestPriorityQueue(void) queue.Dequeue(*msgHigh[3]); VerifyPriorityQueueContent(queue, 0); - // Change the priority of an already queued message and check the order change in the queue. + // Validate that priority of an already queued message in a priority queue cannot change queue.Enqueue(*msgNor[0]); VerifyPriorityQueueContent(queue, 1, msgNor[0]); queue.Enqueue(*msgHigh[0]); @@ -272,54 +239,19 @@ void TestPriorityQueue(void) queue.Enqueue(*msgLow[0]); VerifyPriorityQueueContent(queue, 3, msgHigh[0], msgNor[0], msgLow[0]); - SuccessOrQuit(msgNor[0]->SetPriority(Message::kPriorityNet)); - VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]); - SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityLow)); - VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]); - SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityNormal)); - VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]); - SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityHigh)); - VerifyPriorityQueueContent(queue, 3, msgNor[0], msgHigh[0], msgLow[0]); - SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityNet)); - VerifyPriorityQueueContent(queue, 3, msgNor[0], msgLow[0], msgHigh[0]); - SuccessOrQuit(msgNor[0]->SetPriority(Message::kPriorityNormal)); - SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityLow)); + VerifyOrQuit(msgNor[0]->SetPriority(Message::kPriorityNet) == kErrorInvalidState); + VerifyOrQuit(msgLow[0]->SetPriority(Message::kPriorityLow) == kErrorInvalidState); + VerifyOrQuit(msgLow[0]->SetPriority(Message::kPriorityNormal) == kErrorInvalidState); + VerifyOrQuit(msgHigh[0]->SetPriority(Message::kPriorityNormal) == kErrorInvalidState); VerifyPriorityQueueContent(queue, 3, msgHigh[0], msgNor[0], msgLow[0]); - messageQueue.Enqueue(*msgNor[1]); - messageQueue.Enqueue(*msgHigh[1]); - messageQueue.Enqueue(*msgNet[1]); - VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]); - - // Change priority of message and check for not in messageQueue. - SuccessOrQuit(msgNor[1]->SetPriority(Message::kPriorityNet)); - VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]); - - SuccessOrQuit(msgLow[0]->SetPriority(Message::kPriorityHigh)); - VerifyPriorityQueueContent(queue, 3, msgHigh[0], msgLow[0], msgNor[0]); - VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]); - - // Remove messages from the two queues + // Remove messages from the queue queue.Dequeue(*msgHigh[0]); - VerifyPriorityQueueContent(queue, 2, msgLow[0], msgNor[0]); - VerifyMsgQueueContent(messageQueue, 3, msgNor[1], msgHigh[1], msgNet[1]); - - messageQueue.Dequeue(*msgNet[1]); - VerifyPriorityQueueContent(queue, 2, msgLow[0], msgNor[0]); - VerifyMsgQueueContent(messageQueue, 2, msgNor[1], msgHigh[1]); - - messageQueue.Dequeue(*msgHigh[1]); - VerifyPriorityQueueContent(queue, 2, msgLow[0], msgNor[0]); - VerifyMsgQueueContent(messageQueue, 1, msgNor[1]); - + VerifyPriorityQueueContent(queue, 2, msgNor[0], msgLow[0]); queue.Dequeue(*msgLow[0]); VerifyPriorityQueueContent(queue, 1, msgNor[0]); - VerifyMsgQueueContent(messageQueue, 1, msgNor[1]); - queue.Dequeue(*msgNor[0]); VerifyPriorityQueueContent(queue, 0); - messageQueue.Dequeue(*msgNor[1]); - VerifyMsgQueueContent(messageQueue, 0); for (Message *message : msgNor) {