Initial developments for cubiscan #1
Conversation
jcoux
changed the title
|
|
||
| def get_error(value): | ||
| value_str = decode(value) | ||
| if value_str == 'C': |
There was a problem hiding this comment.
for all check no need to do
| if value_str == 'C': | |
| if upper(value_str) == 'C': |
?
There was a problem hiding this comment.
No. The Documentation of this thing specified that we either get C, M, Z we really shouldn't get anything else. If we get small letters then something went wrong.
There was a problem hiding this comment.
thanks for answer \o/
Can you put this information in the method description ?
README.rst
Outdated
| CUBISCAN | ||
| ======== | ||
|
|
||
| Provides an interface to a cubiscan machine. |
There was a problem hiding this comment.
add a link to https://cubiscan.com/ and mention briefly what we are talking about
| self.response_mappings[name] = (response, neg_response) | ||
|
|
||
| def build_command_string(self, name, params=None): | ||
| """Wrap command and params with the base stuff arround it. |
There was a problem hiding this comment.
worth to mention here the schema of the command as reported on the docs.
<STX><COMMANDE><DONNÉES><ETX><CR><LF> + ACK/NACK variants, etc
| self.add_command('weight_unit', bytes('#', 'ascii'), [], []) | ||
| self.add_command('zero', bytes('Z', 'ascii'), [], []) | ||
|
|
||
| def add_command(self, name, command_bit, response, neg_response): |
There was a problem hiding this comment.
just asking: do you really need this? The list of commands seems pretty static. You could simply have a mapping for commands and a mapping for response, no?
There was a problem hiding this comment.
I figured that another version might have more commands. Also i excluded the ones to change ip, subnetmask and gateway since it can stop you from having access to the device. but someone might want it anyway but otherwise yes i could just have a mapping.
cubiscan/cubiscan.py
Outdated
| command_string = get_command_registry().build_command_string( | ||
| command, param | ||
| ) | ||
| conn = socket.create_connection((self.ip_address, self.port)) |
There was a problem hiding this comment.
would be nice to have a ctx manager
| res_dict = {} | ||
| # we split the response by commata because they arent interesting but | ||
| # they split the response for dimensions etc | ||
| sections = data.split(bytes(',', 'ascii')) |
There was a problem hiding this comment.
seems we can have numbers w/ commas. Am I wrong? If not, is this ok here?
There was a problem hiding this comment.
according to the command table floats are seperated by . not by , so yes it is ok.
| return self._make_request('set_factor', param) | ||
|
|
||
| def set_location(self, location): | ||
| if len(location) < 6: |
| return self._make_request('measure') | ||
|
|
||
| def calibrate_scale(self, weight): | ||
| if not weight > 50 and weight < 100: |
tests/test_parse_response.py
Outdated
|
|
||
|
|
||
| def test_parse_positive_measure_response(): | ||
| scanner_obj = CubiScan('127.0.0.1', '1234') |
There was a problem hiding this comment.
scanner obj is always the same, you could define it globally or define a unit test case
|
@simahawk updated the pr. |
| CUBISCAN | ||
| ======== | ||
|
|
||
| Provides an interface to a cubiscan devices (https://cubiscan.com/). |
| """Wrap command and params with the base stuff arround it. | ||
| """ | ||
| # All commands always have the following format | ||
| # <STX><COMMAND><DATA><ETX><CR><LF> |
There was a problem hiding this comment.
what's the meaning of each block? 👨🏫
| res_dict = {} | ||
| # we split the response by commata because they arent interesting but | ||
| # they split the response for dimensions etc | ||
| sections = data.split(bytes(',', 'ascii')) |
| def _parse_response(self, command, data): | ||
| """Parse the response.""" | ||
| mapping, neg_mapping = get_command_registry().get_response_for(command) | ||
| used_map = mapping |
There was a problem hiding this comment.
re-reading: you only need a reference, fine :)
|
@jcoux @Tonow-c2c @simahawk this pr got closed accidentally when i pushed the wrong remote so i opened #2 if there is more discussion |
No description provided.