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

61 receive responses from a tcp server 2 #74

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

calebbourg
Copy link
Collaborator

Description

describe the intent of your changes here and/or what problem this tries to solve

GitHub Issue: link your GitHub issue here

Changes

  • ...
  • ...
  • ...

Testing Strategy

describe how you or someone else can test and verify the changes

Concerns

describe any concerns that might be worth mentioning or discussing

@calebbourg calebbourg changed the base branch from main to more_robust_wait_response_cmd February 17, 2023 15:23
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
Base automatically changed from more_robust_wait_response_cmd to main February 17, 2023 15:59
@jhodapp jhodapp added the feature work Specifically implementing a new feature label Feb 17, 2023
@jhodapp jhodapp added this to the 0.4 Release milestone Feb 17, 2023
@calebbourg calebbourg force-pushed the 61-receive-responses-from-a-tcp-server_2 branch from 96834b0 to cc4c417 Compare February 17, 2023 18:19
@@ -392,7 +393,7 @@ pub(crate) trait ProtocolInterface {
fn get_conn_status(&mut self) -> Result<ConnectionStatus, Error>;
fn set_dns_config(&mut self, dns1: IpAddress, dns2: Option<IpAddress>) -> Result<(), Error>;
fn req_host_by_name(&mut self, hostname: &str) -> Result<u8, Error>;
fn get_host_by_name(&mut self) -> Result<[u8; MAX_NINA_RESPONSE_LENGTH], Error>;
fn get_host_by_name(&mut self) -> Result<NinaResponseBuffer, Error>;
Copy link
Member

@jhodapp jhodapp Feb 19, 2023

Choose a reason for hiding this comment

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

One idea of mine, perhaps we could have a couple of different NinaResponseBuffer types:

  1. A shorter one, 255 bytes in length and maybe called SmallNinaResponseBuffer
  2. A larger one intended for things like HTTP response documents and maybe called LargeNinaResponseBuffer

The type that we always specify in these method definitions would implement a NinaResponseBuffer trait and then we could make the return type be a generic.

// return Err(ProtocolError::TooManyParameters.into());
// }

let mut response_param_buffer: NinaResponseBuffer = [0; MAX_NINA_RESPONSE_LENGTH];
Copy link
Member

Choose a reason for hiding this comment

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

And then per my comment above about a generic NinaResponseBuffer trait/type, this could instantiate a LargeNinaResponseBuffer.


fn receive_data(&mut self, socket: Socket) -> Result<NinaResponseBuffer, Error> {
self.avail_data_tcp(socket);
let mut response_param_buffer: NinaResponseBuffer = [0; MAX_NINA_RESPONSE_LENGTH];
Copy link
Member

Choose a reason for hiding this comment

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

And per my comment above, this would instantiate a SmallNinaResponseBuffer.

@calebbourg calebbourg force-pushed the 61-receive-responses-from-a-tcp-server_2 branch from a8888fd to 55a4fac Compare February 23, 2023 04:13
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
esp32-wroom-rp/src/spi.rs Fixed Show fixed Hide fixed
operation.command as u8
);
self.control_pins.esp_deselect();
return e;

Check warning

Code scanning / clippy

unneeded `return` statement

unneeded `return` statement
let (current_length, response_buffer) =
self.get_data_buf_tcp(socket, available_data_length)?;

for i in 0..(current_length - 1) {

Check warning

Code scanning / clippy

the loop variable `i` is only used to index `response_buffer`

the loop variable `i` is only used to index `response_buffer`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature work Specifically implementing a new feature
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants