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.

Sign up for GitHub

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

Jump to bottom

MQTT example for SWIOT1L and a couple fixes #2180

Merged
merged 7 commits into from
May 28, 2024
Merged

MQTT example for SWIOT1L and a couple fixes #2180

merged 7 commits into from
May 28, 2024

Conversation

CiprianRegus
Copy link
Contributor

Pull Request Description

  • Adding an MQTT example for the SWIOT1L project. This is an alternative to the current default firmware.
  • The UART parity error flag was sometimes randomly set for the Maxim platform (probably due to noise on the UART bus). Since it was not cleared in either no-OS or the msdk, the interrupt would continuously trigger.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the Coding style guidelines
  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe etc), if applies

@CiprianRegus CiprianRegus force-pushed the staging/swiot1l_mqtt_example branch 2 times, most recently from c1cdc20 to bb8794b Compare May 14, 2024 10:14
@CiprianRegus
Copy link
Contributor Author

I'll have to update the documentation as well

rbolboac
rbolboac reviewed May 14, 2024
View reviewed changes
Copy link
Contributor

@rbolboac rbolboac left a comment

Choose a reason for hiding this comment

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

some remarks from my side, also it looks like the build is failing because the max UART flags are not found

network/lwip_raw_socket/lwip_socket.c
*/
static err_t lwip_connect_callback(void *arg, struct tcp_pcb *pcb, err_t err)
{
struct lwip_socket_desc *sock = arg;
Copy link
Contributor

Choose a reason for hiding this comment

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

seeing how these APIs are static, I am not sure if we should check for null pointers...I suppose the upper layer is taking care of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The upper layer is not checking arg. I've added the check in the lwip_connect_function, however this pointer is shared between multiple callbacks (lwip_accept_callback(), lwip_recv_callback()) and an invalid value will result in a fatal error (meaning the lwip stack won't be able to recover from this and any action such as connect, accept, packet RX, etc will be broken from that point on). So, it's not much different that a null pointer access.

network/lwip_raw_socket/lwip_socket.c Outdated
Comment on lines 859 to 861
ret = tcp_connect(pcb, &ipaddr, addr->port, lwip_connect_callback);
if (ret)
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

can return directly

projects/swiot1l/src.mk Outdated

endif


Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra blank line

projects/swiot1l/src/examples/swiot1l-mqtt/swiot1l_mqtt.c Outdated
};
struct lwip_network_desc *lwip_desc;
struct tcp_socket_desc *tcp_socket;
// struct connection *client_socket;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented line

projects/swiot1l/src/examples/swiot1l-mqtt/swiot1l_mqtt.c Outdated
Comment on lines 174 to 175
if (ret)
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

any memory leaks we should handle?
also, in case of returning, especially if !0, maybe it would be a good idea to print some error messages which would explain what has happened?

@CiprianRegus
drivers: platform: maxim: Clear UART error flags
927a7de 
The UART error flags are currently not cleared in the SDK. Thus, if any
of them are set, the UART interrupt will be continuosly triggered.

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
@CiprianRegus CiprianRegus marked this pull request as draft May 22, 2024 13:32
@CiprianRegus CiprianRegus marked this pull request as ready for review May 22, 2024 14:31
@CiprianRegus
network: lwip_raw_sockets: Implement TCP client specific functions
ec458a7 
Implement the connect() and disconnect() TCP client functionality.

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
@CiprianRegus
libraries: mqtt: Fix data types
9e08e12 
Usually, the payload field of the mqtt_message struct is used in order
to pass "string" data (either a static char* or a buffer formatted
using sprintf). char* and int8_t* may not be equivalent, since on
different CPU architectures char might be defined as either signed or
unsigned, thus a compilation warning is generated. Avoid the need to
cast by using the expected data type.

Apply similar changes for other fields.

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
@CiprianRegus
libraries: mqtt: Adapt the MQTT No-OS layer for the LWIP implementation
2c9ebd6 
Currently, all devices (ADIN1110 and ADIN2111) which may be used with
the LWIP sockets layer require polling for new frames. Add a poll before
socket_recv().

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
@CiprianRegus
driver: adc-dac: ad74413r: Change conversion result type to signed
3388ebe 
When the AD74413R's ADC is measuring current in the voltage output mode,
the result may be signed. A negative current indicates the current is
sourced by the device and a positive one indicates current is sinked.
Currently, since the decimal part of the result is declared as unsigned,
there is no way to represent current values in the (-1:0) mA range.

Fix this by declaring the decimal part as signed.

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
@CiprianRegus CiprianRegus force-pushed the staging/swiot1l_mqtt_example branch 3 times, most recently from 9b7e344 to d521ed8 Compare May 27, 2024 15:29
@CiprianRegus
network: lwip_raw_socket: Implement LWIP remove
1f14f30 
Implement the remove function for the LWIP sockets layer.

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
@CiprianRegus
projects: swiot1l: Add MQTT example
b74ae4d 
The project showcases how you may send data over MQTT (using the T1L
ethernet connector on the SWIOT1L board) to a remote broker.

The default IP and port configurations of the broker are assumed to be
192.168.97.1:1883. These may be changed when compiling by setting the
SWIOT1L_MQTT_SERVER_IP and SWIOT1L_MQTT_SERVER_PORT make variables.

Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
rbolboac
rbolboac approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@rbolboac rbolboac left a comment

Choose a reason for hiding this comment

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

lgtm

@CiprianRegus CiprianRegus merged commit 0461f43 into main May 28, 2024
14 checks passed
@CiprianRegus CiprianRegus deleted the staging/swiot1l_mqtt_example branch May 28, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbolboac rbolboac rbolboac approved these changes

@buha buha Awaiting requested review from buha buha is a code owner

@dbogdan dbogdan Awaiting requested review from dbogdan dbogdan is a code owner

@amiclaus amiclaus Awaiting requested review from amiclaus amiclaus is a code owner

@danmois danmois Awaiting requested review from danmois danmois is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@CiprianRegus @rbolboac