search

travisgoodspeed / md380tools

Python tools and patched firmware for the TYT-MD380
804 stars 245 forks source link

get_dmrmarc_json.py: clarify python-request requirment #860

Closed stefansaraev closed 6 years ago

stefansaraev commented 6 years ago

closes #859

@KD4Z are you ok with this ?

KD4Z commented 6 years ago

I'm still in the camp that likes to see errors separated from the data as an exception report. I believe it might be hazardous to have errors tossed into the standard output file. Certainly would confuse the radio if it gets written into the radio that way.

Good show on the move for request inside the Try. I have run into two instances this week, of users having issue with the script failing without request being installed. It seems that Linux Mint comes out of the box without it. We need an addition to either the debian.md or add one for Mint, that adds a prerequisites for python-pip and request. Can you add that as part of your PR?

stefansaraev commented 6 years ago

any python errors (the traceback that comes aout of it) always go into stderr, nothing bad will happen. and for this very special case where I catch the error and print something, I make sure to also print it on stderr.

I could as well not try-catch at all, and it would still be fine. but I decided to print a bit more informative message instead.

I'll add python-requests to readme.md files

EDIT: or we can go with urllib2 (part of python2 stdlib) instead requests

KD4Z commented 6 years ago

Ok, I could a swear you had stderr redirected to $@ That will teach me to look at this stuff on a cell phone display :) Looks good now.

stefansaraev commented 6 years ago

another alternative: https://github.com/stefansaraev/md380tools/commit/7f11387ba4c562f38f51b47c956737a6fcb3b63b

travisgoodspeed commented 6 years ago

Merging from @KD4Z's review. Thanks for the PR, @stefansaraev!