Commented Out iotc_bsp_io_net_socket_connect Function

[galz10]

The call to iotc_bsp_io_net_socket_connect would break the program since, once i commented the function out everything works

[galz10]

@atigyi @hongalex

[gguuss]

LGTM if tests pass, looks like this is occurring externally as well:

https://github.com/espressif/esp-google-iot/issues/10

My impression is that this should be implemented in the board support package (BSP) for Espressif.

[galz10]

It also seems like the second check for Travis never passes , pnfisher also is having this problem

[gguuss]

It also seems like the second check for Travis never passes , pnfisher also is having this problem

I think I have seen the check pass but I'm unsure of why this happens. IIRC on your last PR committing an additional change triggered the check.

[atigyi]

🚫 Not Approved.

@galz10 "once i commented the function out everything works" what does "everything works" mean? Also "The call to iotc_bsp_io_net_socket_connect would break the program since" what would it break? Does it break the build or does it crash the program during runtime? What platform do you use?

This code alters platform independent connection logic. The iotc_bsp_io_net_socket_connect is intended to create a socket and the implementation is platform specific and should be in the specific BSP implementation one aims for.

Regarding the espressif/esp-google-iot#10 issue: Since the ESP port the BSP IO NET interface has changed. They have an implementation on a function which header (name and attributes) has changed. iotc_bsp_io_net_connect --> iotc_bsp_io_net_socket_connect

✅ The solution is to adapt the IO NET BSP's iotc_bsp_io_net_connect function to the new version. The new version btw. supports UDP sockets as well, but this functionality isn't used from inside the lib yet, so the BSP impl. doesn't have to be changed (extended with UDP support). Just the adapt the function name and add the extra parameter to the end and ignore it in the function body. This will fix the build (I assume the build is broken).

[galz10]

🚫 Not Approved.

@galz10 "once i commented the function out everything works" what does "everything works" mean? Also "The call to iotc_bsp_io_net_socket_connect would break the program since" what would it break? Does it break the build or does it crash the program during runtime? What platform do you use?

This code alters platform independent connection logic. The iotc_bsp_io_net_socket_connect is intended to create a socket and the implementation is platform specific and should be in the specific BSP implementation one aims for.

Regarding the espressif/esp-google-iot#10 issue: Since the ESP port the BSP IO NET interface has changed. They have an implementation on a function which header (name and attributes) has changed. iotc_bsp_io_net_connect --> iotc_bsp_io_net_socket_connect

✅ The solution is to adapt the IO NET BSP's iotc_bsp_io_net_connect function to the new version. The new version btw. supports UDP sockets as well, but this functionality isn't used from inside the lib yet, so the BSP impl. doesn't have to be changed (extended with UDP support). Just the adapt the function name and add the extra parameter to the end and ignore it in the function body. This will fix the build (I assume the build is broken).

Sorry for not specifying, the iotc_bsp_io_net_socket_connect breaks the build. I'll adapt the function name to see if it fixes the build. I also saw that instead of using iotc_bsp_io_net_socket_connect Espressif uses iotc_bsp_io_net_connection_check. Using iotc_bsp_io_net_connection_check instead of iotc_bsp_io_net_socket_connect also builds successfully, is that also a good fix ?