Update Example Return Values #317

Jake-Carter · 2023-01-02T23:38:29Z

EdwinFairchild · 2023-01-03T21:09:54Z

should be able to merge main into this for new CI-test.

ozersa · 2023-01-06T13:19:03Z

Examples/MAX32572/AES/main.c

    }

-    while (1) {}
-
    return 0;


This line might cause warning, like unreachable point...

@ozersa and @Jake-Carter Can you build MAX32572 samples? When I import the projects on Eclipse I am seeing "Preprocessor Include Path/Entries/GNU C/CDT Cross GCC Built-in Compiler Settings" as empty. Therefore, the project can not find gcc command. This is why I have skipped MAX32572 samples.

ozersa · 2023-01-06T13:22:34Z

Examples/MAX32520/CRC/main.c

    } else {
        printf("   Passed  \n");
+        return 0;
    }
 }


It would be better just returning result,
The success return value must be consistent (zero) but fail case can be different (non-zero).
Instead of increasing code line just returning result will be better.
Please apply same idea for other cases in this PR.

@ozersa this is an internal function. Moreover, such change may require many changes on other projects. Isn't it better to skip such change in our first iteration and somehow achieve to merge. @Jake-Carter what do you think?

@karaanil I agree with @ozersa that this small internal function is probably not necessary.

If you really wanted to inform the user which CRCs are failing, I would suggest a simple printf like below.

int Test_CRC(int asynchronous) { // ... ret = (CHECK != crc_req.resultCRC); if (ret) printf( "Failed CRC check! Received: %x\tExpected: %x\n", crc_req.resultCRC, CHECK); MXC_CTB_Shutdown(MXC_CTB_FEATURE_CRC | MXC_CTB_FEATURE_DMA); return ret; }

It looks like only the CRC examples would need this cleanup

ozersa · 2023-01-06T13:23:53Z

Examples/MAX32572/CRC/main.c

    } else {
        printf("   Passed  \n");
+        return 0;
    }
 }


Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063430730

ozersa · 2023-01-06T13:26:00Z

Examples/MAX32572/DES/main.c


-    printf("\nExample complete.\n");
+    return 0;


Might give warning (unreachable line...), please check it.

ozersa · 2023-01-06T13:28:27Z

Examples/MAX32572/DMA/main.c

    }

-    while (1) {}
-
    return 0;


Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063432996

ozersa · 2023-01-06T13:29:02Z

Examples/MAX32572/Hash/main.c

    } else {
        printf("   Passed  \n\n");
+        return 0;


Same as above comment...

ozersa · 2023-01-06T13:29:47Z

Examples/MAX32572/Hash/main.c

+    } else {
+        printf("Example Failed\n");
+        return -1;
+    }

    return 0;


Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063432996

ozersa · 2023-01-06T13:31:41Z

Examples/MAX32572/SDHC_Raw/main.c

+        return -1;
+    }
+
+    return 0;


Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063432996

ozersa · 2023-01-06T13:32:01Z

Examples/MAX32572/SPIXF/main.c

    } else {
        printf("Example Failed\n\n");
+        return -1;
    }
    return 0;


Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063432996

ozersa · 2023-01-06T13:33:32Z

Examples/MAX32572/SPIXR/main.c

    }

-    while (1) {}
+    return 0;


Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063432996
Please apply it for all case in this PR

ozersa · 2023-01-06T13:37:33Z

Examples/MAX32655/CRC/main.c

@@ -140,11 +140,12 @@ int main(void)

    if (fail) {
        printf("\Example Failed");


Not related this PR but please fix typo add \n,
\E might cause some issue

ozersa · 2023-01-06T13:43:32Z

Examples/MAX32660/UART/main.c

    } else {
        printf("-->EXAMPLE SUCCEEDED\n");
    }

-    while (1) {


The PR shall not change example functionality (as possible as),
In this case at the end of example led blinks, instead of removing this feature you should add a counter to blink the led n times, n can be 10.

@ozersa which line blinks the LED? I am seeing that it is either off or on. Since GPIO's will preserve the state after code finishes, it should work as before. Am I wrong?

It is okay that we have such a while loop at the end. However, I am checking the code and it initially has blocking empty while loops at the top that changed to a simple return. I mean this portion mostly is not executed. Moreover, the other UART examples do not have such a loop in their original form. Therefore, removing this portion makes this example similar to the other UART examples.
Do we really need such a portion? Why others do not have it but MAX32660 has? @ozersa

ozersa · 2023-01-06T13:44:34Z

Examples/MAX32662/UART/main.c

@@ -218,15 +218,10 @@ int main(void)
    if (fail != 0) {
        LED_On(0);
        printf("-->EXAMPLE FAILED\n");
-        while (1) {}
+        return E_FAIL;
    } else {
        printf("-->EXAMPLE SUCCEEDED\n");
    }



Same as
https://github.com/Analog-Devices-MSDK/msdk/pull/317/files#r1063445170

ozersa · 2023-01-06T14:03:09Z

Examples/MAX78000/AES/main.c

@@ -45,6 +45,7 @@
 #include "board.h"
 #include "dma.h"
 #include "aes.h"
+#include "aes_regs.h"


Not needed to include aes_regs.h file
aes.h file include it.

ozersa · 2023-01-06T14:04:11Z

Examples/MAX32690/Flash/main.c

@@ -47,6 +47,7 @@
 #include "nvic_table.h"
 #include "flc.h"
 #include "icc.h"
+#include "flc_regs.h"


No need to include flc_regs.h file flc.h already include it.

ozersa · 2023-01-06T14:05:08Z

Examples/MAX32690/DMA/main.c

@@ -45,6 +45,7 @@
 #include "mxc_device.h"
 #include "nvic_table.h"
 #include "dma.h"
+#include "dma_regs.h"


No need to include dma_regs.h file dma.h already include it.

ozersa · 2023-01-06T14:06:22Z

Examples/MAX78000/DMA/main.c

@@ -45,6 +45,7 @@
 #include "mxc_device.h"
 #include "nvic_table.h"
 #include "dma.h"
+#include "dma_regs.h"


No need to include dma_regs.h file. dma.h already include it.

ozersa · 2023-01-06T14:07:29Z

Examples/MAX78002/AES/main.c

@@ -45,6 +45,7 @@
 #include "board.h"
 #include "dma.h"
 #include "aes.h"
+#include "aes_regs.h"


No need to include aes_regs.h file.

ozersa · 2023-01-06T14:07:50Z

Examples/MAX78002/DMA/main.c

@@ -45,6 +45,7 @@
 #include "mxc_device.h"
 #include "nvic_table.h"
 #include "dma.h"
+#include "dma_regs.h"


no needed include.

ozersa · 2023-01-06T14:08:39Z

Examples/MAX78002/Flash/main.c

@@ -47,6 +47,7 @@
 #include "nvic_table.h"
 #include "flc.h"
 #include "icc.h"
+#include "flc_regs.h"


Same as above.

ozersa

Unneeded code lines and includes should not be added
The example functionality shall not be change as possible as.

Standardize return message and value for single shot ones Signed-off-by: Sadik.Ozer <sadik.ozer@analog.com>

ozersa

Related changes has been applied.

ozersa · 2023-01-19T12:09:35Z

@Jake-Carter Could it be merged as soon as possible?
The PR effect too many examples so that if it not been merged in a short term, it will be out of date and more conflict will occur in future that will cause extra rework...
I will appreciate if it to be merged as soon as possible.

Jake-Carter · 2023-01-19T21:45:38Z

@ozersa Of course, just merged

Jake-Carter mentioned this pull request Jan 2, 2023

Update Return Values #218

Closed

Jake-Carter requested review from karaanil and ozersa January 2, 2023 23:43

karaanil approved these changes Jan 5, 2023

View reviewed changes

ozersa reviewed Jan 6, 2023

View reviewed changes

Examples/MAX32572/Hash/main.c Outdated

} else {

printf(" Passed \n\n");

return 0;

Copy link

Contributor

ozersa Jan 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above comment...

ozersa reviewed Jan 6, 2023

View reviewed changes

ozersa requested changes Jan 6, 2023

View reviewed changes

karaanil added 4 commits January 19, 2023 14:21

[MSDK-911]MAX32520 return value update

570ed68

[MSDK-918] MAX32572 return value update

75288b6

[MSDK-919] MAX32650 return value update

fc605c9

[MSDK-920] MAX32655 return value update

682f989

karaanil and others added 13 commits January 19, 2023 14:21

[MSDK-921] MAX32660 return value update

6f41b36

[MSDK-922] MAX32662 return value update

56561f6

[MSDK-923] MAX32665 return value update

e82b729

[MSDK-924] MAX32670 return value update

3ef71c9

[MSDK-925] MAX32672 return value update

1071fdc

[MSDK-926] MAX32675 return value update

3c7a50f

[MSDK-935] MAX32680 return value update

48b194c

[MSDK-937 ]MAX32690 return value update

5f25885

[MSDK-938] MAX78000 return value update

7723dd8

[MSDK-939] MAX78002 return value update

3b8a311

clang-format bot reformatting.

e06b463

[MSDK-987] Use mxc_errors for return values

f951246

clang-format bot reformatting.

bbf943a

ozersa force-pushed the dev-examples-return-value-update branch from 8cfcc0b to cabb35b Compare January 19, 2023 11:22

MSDK-989: Review examples

18a33e1

Standardize return message and value for single shot ones Signed-off-by: Sadik.Ozer <sadik.ozer@analog.com>

ozersa force-pushed the dev-examples-return-value-update branch from cabb35b to 18a33e1 Compare January 19, 2023 11:39

ozersa approved these changes Jan 19, 2023

View reviewed changes

Jake-Carter merged commit 2591919 into main Jan 19, 2023

Jake-Carter deleted the dev-examples-return-value-update branch January 19, 2023 21:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Example Return Values #317

Update Example Return Values #317

Jake-Carter commented Jan 2, 2023

EdwinFairchild commented Jan 3, 2023

ozersa Jan 6, 2023

karaanil Jan 9, 2023

ozersa Jan 6, 2023

karaanil Jan 9, 2023

Jake-Carter Jan 12, 2023

Jake-Carter Jan 12, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023 •

edited

Loading

ozersa Jan 6, 2023

karaanil Jan 9, 2023

ozersa Jan 9, 2023

karaanil Jan 9, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa Jan 6, 2023

ozersa left a comment

ozersa left a comment

ozersa commented Jan 19, 2023

Jake-Carter commented Jan 19, 2023

		@@ -140,11 +140,12 @@ int main(void)

		if (fail) {
		printf("\Example Failed");

Update Example Return Values #317

Update Example Return Values #317

Conversation

Jake-Carter commented Jan 2, 2023

EdwinFairchild commented Jan 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ozersa Jan 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ozersa left a comment

Choose a reason for hiding this comment

ozersa left a comment

Choose a reason for hiding this comment

ozersa commented Jan 19, 2023

Jake-Carter commented Jan 19, 2023

ozersa Jan 6, 2023 •

edited

Loading