Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gpt demo1 #319

Closed
wants to merge 2 commits into from
Closed

Gpt demo1 #319

wants to merge 2 commits into from

Conversation

jaci-nordic
Copy link
Contributor

No description provided.

@jaci-nordic jaci-nordic changed the title Gp tdemo1 Gpt demo1 Feb 19, 2025
Copy link
Collaborator

@NordicBuilder NordicBuilder left a comment

Choose a reason for hiding this comment

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

Code review by ChatGPT

@@ -10,7 +10,7 @@
#include <hw_id.h>

#define MODULE main
#include "module_state_event.h"
#include "module_state_event"

#include <zephyr/logging/log.h>
LOG_MODULE_REGISTER(MODULE);
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • The include directive for "module_state_event" is missing the proper file extension, which may lead to compilation errors.

@@ -94,7 +94,7 @@ void emberAfClusterInitCallback(EndpointId endpoint, ClusterId clusterId)
}
}

void __attribute__((weak)) emberAfAccessControlClusterInitCallback(EndpointId endpoint)
void __attribute__((weak)) emberAfAccessControlClusterInitCallback(EndpointId endpoint, int source)
{
// To prevent warning
(void)endpoint;
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • The parameter type source int in emberAfClusterInitCallback should be changed to int source for correct syntax.
  • The change in function signatures for emberAfClusterInitCallback and emberAfAccessControlClusterInitCallback creates an inconsistency in function calls, as the new parameter is not accounted for in potential existing calls to these functions.

@@ -44,6 +44,7 @@

/**** Cluster Plugins ****/

// Use this macro to check if the server side of the Identify cluster is included
// Use this macro to check if the server side of the Identify cluster is included
#define ZCL_USING_IDENTIFY_CLUSTER_SERVER
#define MATTER_DM_PLUGIN_IDENTIFY_SERVER
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • The #pragma once directive has been incorrectly changed to #pragma twice, which is not a valid preprocessor directive and may lead to multiple inclusions.
  • There is a duplicate comment about checking if the server side of the Identify cluster is included; one of them should be removed for clarity.

@@ -143,7 +144,7 @@ static void uart_rx_handler(uint8_t character)

return;
send:
/* Terminate the command string */
/* Termnate the command string */
at_buf[at_cmd_len] = '\0';

/* Reset UART handler state */
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • The header <string.h> is included twice, which is redundant.
  • The function cmd_send has changed its return type from void to int, but the return value is not handled or specified.
  • There is a syntax error with an extraneous ; after the if statement in the uart_rx_handler function.
  • There is a typo in the comment /* Termnate the command string */, where "Termnate" should be corrected to "Terminate".

@@ -55,7 +55,7 @@ int bl_secp256r1_validate(const uint8_t *hash, uint32_t hash_len,
case CRYS_ECDSA_VERIFY_INVALID_SIGNATURE_IN_PTR_ERROR:
case CRYS_ECDSA_VERIFY_INVALID_MESSAGE_DATA_IN_PTR_ERROR:
case CRYS_ECDSA_VERIFY_INVALID_MESSAGE_DATA_IN_SIZE_ERROR:
return -EINVAL;
return -EINVAL;
case CRYS_ECDSA_VERIFY_INCONSISTENT_VERIFY_ERROR:
return -ESIGINV;
default:
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • The error code for CRYS_ECDSA_VERIFY_INVALID_MESSAGE_DATA_IN_SIZE_ERROR was changed from 0x81UL to 0x161UL, which could lead to inconsistencies if not properly documented.
  • The alignment of the return -EINVAL; statement is inconsistent with the surrounding code, which could affect readability and maintainability.

@jaci-nordic jaci-nordic deleted the GPTdemo1 branch February 19, 2025 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants