Hwdetect task#109
Conversation
AleksandrBulyshchenko
left a comment
AleksandrBulyshchenko
left a comment
There was a problem hiding this comment.
@marynamalakhova,
In general - OK,
but see my comments inline.
hwdetect/hwdetect.sh
Outdated
| @@ -1,4 +1,54 @@ | |||
| #!/bin/sh | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Changing interpreter deserves at least to be mentioned in the commit message (with reasoning).
hwdetect/hwdetect.sh
Outdated
| exit 0 | ||
| ;; | ||
| *) echo "Invalid option $REPLY";; | ||
| esac |
|
|
||
| function find_sdcard() | ||
| { | ||
| echo "SDcards" | ||
| } | ||
|
|
||
| function find_i2c() | ||
| { | ||
| echo "i2c" | ||
| echo $(ls /dev | grep i2c) | ||
| } | ||
|
|
||
| function find_usb_ttl() | ||
| { | ||
| echo "usb_ttl" | ||
| echo $(ls /dev | grep tty) | ||
| } | ||
|
|
||
| function find_flash() | ||
| function find_sd_card() | ||
| { | ||
| echo "flash" | ||
| echo "sd card" | ||
| } |
There was a problem hiding this comment.
Commit f80508a states that it only implements find_flash(), but actually it contains implementation of find_i2c(), find_usb_ttl() and renaming find_sdcard().
Commit message should reflect actual changes.
But better to really separate not connected changes into different patches.
hwdetect/hwdetect.sh
Outdated
| echo "Please install it before" | ||
| return -1 | ||
| fi | ||
| lsblk -rno SIZE,NAME,HOTPLUG,MOUNTPOINT | grep -w 1| grep -vE "sr|loop" | awk '{if ($4!="") print $1," ",$4}' |
There was a problem hiding this comment.
Neither commit message nor code explains why such not obvious processing (like grep -w 1 and others) is applied.
(also please don't forget to separate pipes with spaces)
| mapfile -t i2c_array < <(ls /dev | grep i2c) | ||
| for i in ${i2c_array[@]}; do | ||
| echo $i | ||
| done |
There was a problem hiding this comment.
- inconsistent indentation;
Not mistake, but I'm just curious why arrays are used here if the results are actually not processed in any way which requires it?
There was a problem hiding this comment.
I decided that it could be processed later, but I had no opportunity to check the workflow of this part of the script on my laptop. Probably I`ll check it later on the board.
hwdetect/hwdetect.sh
Outdated
| echo "sd card" | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Please apply such changes inplace to not pollute functional changes.
hwdetect/hwdetect.sh
Outdated
| (ls /dev | grep mmcblk) > newsdcard | ||
| lsusb | grep -i storage >> newsdcard | ||
| diff sdcard newsdcard | awk -F'>' '{if ($2!="") print "Added ", $2}' | ||
| diff sdcard newsdcard | awk -F'<' '{if ($2!="") print "Removed ", $2}' | ||
| cat newsdcard > sdcard |
There was a problem hiding this comment.
Creation files in working directory when user doesn't expect this is definitely bad idea.
You should use /tmp/, etc. for such purpose.
Or if you don't need to preserve state between launches - files aren't needed at all - variables should be enough.
BTW, have you seen lsusb showing device with "storage" in description?
Is it block device?
(Because in my case only cardreader itself is seen and it doesn't have "storage" in description)
There was a problem hiding this comment.
I have no SDcard reader in GL laptop, but I have separate SDCard reader via usb and it is detected as usb staorage device. It is observed as 'sda' but not 'mmcblk'
AleksandrBulyshchenko
added
Change requested
marynamalakhova
force-pushed
the
hwdetector
branch
4 times, most recently
from
14d07b9 to
0b2d3d2
Compare
|
Thank you for review. I`ve fixed |
marynamalakhova
added
Ready for review
marynamalakhova
force-pushed
the
hwdetector
branch
from
0b2d3d2 to
ec33c71
Compare
Add util main menu. There are for options. There is proposition to find the list of: -USB to TTL convertors, -flash drives, -SD cards, -i2c devices Signed-off-by: Maryna Malakhova <maryna.malakhova@globallogic.com>
Add function to detect Flash disks via lsblk tool Filter lsblk result by grep command and awk script in order to get userfriendly output Signed-off-by: Maryna Malakhova <maryna.malakhova@globallogic.com>
Add i2c slot lister Signed-off-by: Maryna Malakhova <maryna.malakhova@globallogic.com>
Add usb-serial converter but without /dev/ttyUSBn Signed-off-by: Maryna Malakhova <maryna.malakhova@globallogic.com>
Add SD card finder via mmcblk checking in /dev and looking for usb Signed-off-by: Maryna Malakhova <maryna.malakhova@globallogic.com>
marynamalakhova
force-pushed
the
hwdetector
branch
2 times, most recently
from
1e0b49e to
09e2349
Compare
2 participants
Hardware detector bash script implemented