Hi Giovanni,
I have collected changes for Mails, Timers, Pools.
In attachment, you can find them.
Thanks,
cmsis_os add MailQueue
-
- 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
- Attachments
-
- cmsis_os_Fix_MailBox_Pools_Timers.zip
- (8.34 KiB) Downloaded 159 times
-
- 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
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.
Could you please take a look at that?
Thanks.
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.
- 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
Good point, the increment should be within the loop.
I will open a ticket about this.
Giovanni
I will open a ticket about this.
Giovanni
- 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
Hi,
The issue with the reset function should be solved, fixed as bug #1093.
Giovanni
The issue with the reset function should be solved, fixed as bug #1093.
Giovanni
- 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:
-
- 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
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:
Changes are in the attachment.
Attached version Now PASS all Mail tests.
Thanks.
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
- 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
Hi Bogdan,
What is the point of this change?
to
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
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
-
- 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
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,
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
-
- 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
Hi Giovanni,
I have attached one more variant with a switchable pool range check during freeing memory.
Thanks,
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 7 guests