cmsis_os add MailQueue

Use this forum for requesting small changes in ChibiOS. Large changes should be discussed in the development forum. This forum is NOT for support.
Bogdan
Posts: 21
Joined: Fri Apr 08, 2016 4:33 pm
Location: Ukraine Lviv
Has thanked: 2 times
Been thanked: 3 times
Contact:

Re: cmsis_os add MailQueue

Postby Bogdan » Thu Apr 30, 2020 12:39 pm

Hi Giovanni,

I have collected changes for Mails, Timers, Pools.
In attachment, you can find them.

Thanks,
Attachments
cmsis_os_Fix_MailBox_Pools_Timers.zip
(8.34 KiB) Downloaded 158 times

Bogdan
Posts: 21
Joined: Fri Apr 08, 2016 4:33 pm
Location: Ukraine Lviv
Has thanked: 2 times
Been thanked: 3 times
Contact:

Re: cmsis_os add MailQueue

Postby Bogdan » Thu Apr 30, 2020 12:56 pm

Hi,

Also During Validating Semaphores, I faced with the issue.
In the scenario below, we have an incorrect behavior of chSemReset.

chSemObjectInit(sem, 0x7FFFFFFF); // 0x7FFFFFFF (2,147,483,647) -is the maximum possible positive value for int32_t
chSemReset(sem, 0);

The chSemReset(sem, 0); in this case pre-increment counter in the while loop. And because overflow we got negative value. And trying to remove thread from the queue.
while (++cnt <= (cnt_t)0) {
chSchReadyI(queue_lifo_remove(&sp->queue))->u.rdymsg = MSG_RESET;
}


Could you please take a look at that?

Thanks.

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: cmsis_os add MailQueue

Postby Giovanni » Thu Apr 30, 2020 4:10 pm

Good point, the increment should be within the loop.

I will open a ticket about this.

Giovanni

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: cmsis_os add MailQueue

Postby Giovanni » Fri May 01, 2020 11:23 am

Hi,

The issue with the reset function should be solved, fixed as bug #1093.

Giovanni

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: cmsis_os add MailQueue

Postby Giovanni » Sat May 02, 2020 6:22 pm

Hi,

Patch committed with some more documentation fixes.

Giovanni

Bogdan
Posts: 21
Joined: Fri Apr 08, 2016 4:33 pm
Location: Ukraine Lviv
Has thanked: 2 times
Been thanked: 3 times
Contact:

Re: cmsis_os add MailQueue

Postby Bogdan » Sat May 02, 2020 10:38 pm

Hi,

I have updated some return codes to be able to pass tests.
Also, I have modified osMailQId to be able to check the situation when the code of Business Logic returns the address in osMailFree that was not allocated before from Mail. Was required by the test scenario:
uint32_t val = 0;
ASSERT_TRUE (osMailFree (MailQ_Id, &val) == osErrorValue);


Changes are in the attachment.

Attached version Now PASS all Mail tests.

CMSIS-RTOS Test Suite   May  3 2020   00:17:50 

TEST 01: TC_MailAlloc PASSED
TEST 02: TC_MailCAlloc PASSED
TEST 03: TC_MailToThread PASSED
TEST 04: TC_MailFromThread PASSED
TEST 05: TC_MailTimeout PASSED
TEST 06: TC_MailCheckTimeout PASSED
TEST 07: TC_MailParam PASSED
TEST 08: TC_MailInterrupts PASSED
TEST 09: TC_MailFromThreadToISR PASSED
TEST 10: TC_MailFromISRToThread PASSED

Test Summary: 10 Tests, 10 Executed, 10 Passed, 0 Failed, 0 Warnings.
Test Result: PASSED


Thanks.
Attachments
cmsis_os_MailsFixPassedAll.zip
(8.64 KiB) Downloaded 163 times

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: cmsis_os add MailQueue

Postby Giovanni » Sun May 03, 2020 9:38 am

Hi Bogdan,

What is the point of this change?

Code: Select all

typedef objects_fifo_t *osMailQId;

typedef struct os_mailQ_def {
  uint32_t                  queue_sz;
  uint32_t                  item_sz;
  objects_fifo_t            *fifo;
  msg_t                     *msgbuf;
  void                      *objbuf;
} osMailQDef_t;


to

Code: Select all

typedef struct os_mailQ_id {
  size_t                  queue_sz;
  size_t                  item_sz;
  objects_fifo_t          fifo;
  void                    *objbuf;
} *osMailQId;

typedef struct os_mailQ_def {
  osMailQId                 mailq;
  msg_t                     *msgbuf;
} osMailQDef_t;


You moved constant data (queue_sz, item_sz, objbuf) into a variable, those are never modified after creating the object. If it is just for that extra check then it looks overkill, it takes RAM and slows the function at runtime.

I committed the changes to returned error codes.

Another question, can those test scripts be turned in one of our test suites somehow? without test code this subsystems will not be tested like all others before releases or after changes.

Giovanni

Bogdan
Posts: 21
Joined: Fri Apr 08, 2016 4:33 pm
Location: Ukraine Lviv
Has thanked: 2 times
Been thanked: 3 times
Contact:

Re: cmsis_os add MailQueue

Postby Bogdan » Sun May 03, 2020 1:24 pm

Hi Giovanni,

You moved constant data (queue_sz, item_sz, objbuf) into a variable, those are never modified after creating the object. If it is just for that extra check then it looks overkill, it takes RAM and slows the function at runtime.

Yes, this move was made to add this check in the osMailFree functionality. I agree that it will cost an additional amount of RAM but from my point of view, the reliability of osMailFree functionality is crucial. That's why I made the third version that has only pointer size overhead in comparison from the original version. You can find it in the attachment.
In another case, We can make this part switchable by adding define for enabling this extra check and the additional field in osMailQId.


Another question, can those test scripts be turned in one of our test suites somehow? without test code this subsystems will not be tested like all others before releases or after changes.

I am using in my project directly "ARM.CMSIS-RTOS_Validation" scripts without modifications. From the license point, I do not know that we can copy them or not, because I am using them only locally and in a private repository.
I suppose It can be rewritten totally but in this case, it will involve a lot of manual work.
For the case of integrating scripts "ARM.CMSIS-RTOS_Validation" in the current Chibios testing subsystem, I can't say anything for now it require time to analyze the current testing subsystem.


Thanks,
Attachments
cmsis_os.zip
(8.89 KiB) Downloaded 151 times

Bogdan
Posts: 21
Joined: Fri Apr 08, 2016 4:33 pm
Location: Ukraine Lviv
Has thanked: 2 times
Been thanked: 3 times
Contact:

Re: cmsis_os add MailQueue

Postby Bogdan » Sun May 03, 2020 4:24 pm

Hi Giovanni,

I have attached one more variant with a switchable pool range check during freeing memory.

Thanks,
Attachments
cmsis_os.zip
(8.91 KiB) Downloaded 156 times


Return to “Small Change Requests”

Who is online

Users browsing this forum: No registered users and 13 guests