Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#1759 closed defect (fixed)

3G Support: show connection errors

Reported by: Dcastelo Owned by: Dcastelo
Priority: Unspecified by Maintainer Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Major Keywords: 3G GSM modem usb r+
Cc: tomeu, tch, erikos, garycmartin Distribution/OS: Unspecified
Bug Status: New

Description

This patch improves the feature 3G Support (added in 0.88).
Includes:
Show the connection errors
Apply Eben's Mockup: http://wiki.sugarlabs.org/go/File:3G_device.png

Attachments (13)

0002-Showing-Errors-Palette-Mockup.patch (17.7 KB) - added by Dcastelo 9 years ago.
Ebens Mockup for GSM Connection Palett. Improve feature to show connection errors
Showing Errors-Palette Mockup-V2.patch (16.6 KB) - added by Dcastelo 9 years ago.
Showing-Errors-Pallete-Mockup V3.patch (16.6 KB) - added by Dcastelo 9 years ago.
Minor changes: Change "up-down" icons
0001-Showing-Errors-Pallete-Mockup-V4.patch (16.6 KB) - added by Dcastelo 9 years ago.
Minor changes: Change "connect" and "cancel" icons
0001-Showing-Errors-Pallete-Mockup-V5.patch (23.1 KB) - added by Dcastelo 9 years ago.
Improvements based on comments made by James Cameron
gsm.png (10.4 KB) - added by erikos 9 years ago.
device attached
gsm-error.png (14.9 KB) - added by erikos 9 years ago.
Error message with current code
gsm-error-2.png (19.4 KB) - added by erikos 9 years ago.
Proposal
gsm-error-mockup-gary.png (5.3 KB) - added by garycmartin 9 years ago.
Mock-up suggestion
0001-3G-Support-show-connection-errors-1759.patch (23.3 KB) - added by erikos 9 years ago.
New patch that does address Tomeu's comments and a slightly modified design (after discussion with Gary)
gsm.2.png (10.3 KB) - added by erikos 9 years ago.
Connect
gsm_error_1.png (16.0 KB) - added by erikos 9 years ago.
Error: could not connect / false config
gsm_error_2.png (13.9 KB) - added by erikos 9 years ago.
No connection set in the CP

Download all attachments as: .zip

Change History (30)

comment:1 Changed 9 years ago by Dcastelo

This ticket applies over http://bugs.sugarlabs.org/ticket/1738

comment:2 Changed 9 years ago by Dcastelo

I can't atach the patch:
Submission rejected as potential spam (Akismet says content is spam, Content contained these blacklisted patterns: 'LE*')

comment:3 Changed 9 years ago by sascha_silbe

I've configured Trac not to look at attachment content now. Please try attaching it again.

Changed 9 years ago by Dcastelo

Ebens Mockup for GSM Connection Palett. Improve feature to show connection errors

comment:4 Changed 9 years ago by erikos

  • Bug Status changed from Unconfirmed to New
  • Severity changed from Unspecified to Major
  • Version changed from Unspecified to Git as of bugdate

Hi Daniel,

with landing #1768, your patch needs to be re-spinned to head :/

Did you run pylint and pep8 on the files affected by your patch. I think I saw a wrong indentation... For the NM_ERRORS, I guess I would only add the ones that we actually use. self.conn_info_box, please no abbreviation here.

Regards,

Simon

Changed 9 years ago by Dcastelo

Changed 9 years ago by Dcastelo

Minor changes: Change "up-down" icons

Changed 9 years ago by Dcastelo

Minor changes: Change "connect" and "cancel" icons

comment:5 Changed 9 years ago by erikos

  • Cc erikos added

comment:6 Changed 9 years ago by erikos

  • Milestone changed from 0.88 to 0.88.x

I would suggest it as a bug fix update as Daniel stated that the deployments Uruguay and Paraguay woul dbe interested in getting this in.

comment:7 Changed 9 years ago by tomeu

  • Keywords r! added; r? removed

The patch needs to address some comments made in http://lists.sugarlabs.org/archive/sugar-devel/2010-May/023972.html

comment:8 Changed 9 years ago by Dcastelo

Improves based on comments made by James Cameron (http://lists.sugarlabs.org/archive/sugar-devel/2010-May/023972.html)

Details about the corrections in http://lists.sugarlabs.org/archive/sugar-devel/2010-May/024182.html

Patch: 0001-Showing-Errors-Pallete-Mockup-V5.patch

Changed 9 years ago by Dcastelo

Improvements based on comments made by James Cameron

comment:9 Changed 9 years ago by tch

Just tested "0001-Showing-Errors-Pallete-Mockup-V5.patch" and it works great :) Also looks very nice!

comment:10 Changed 9 years ago by erikos

  • Keywords r? added; r! removed

comment:11 Changed 9 years ago by tomeu

  • Keywords r! added; r? removed

Simon, thanks for correcting the flag.

I think this patch is pretty much ready for merging, with these comments:

  • please run pylint
  • initializing NM_DEVICE_STATE_REASON_DESCRIPTION like in the patch means calling gettext several times at startup. Better do it lazily as needed.

+ if (connection_time is not None):
+ self.props.secondary_text = _('Connected for ' + \
+ connection_time.strftime('%H:%M:%S'))
+ else:
+ self.props.secondary_text = _('Connected for ' \
+ + '00:00:00')

This won't get you the strings translated, try instead:

_('Connected for %s') % connection_time.strftime('%H:%M:%S')

A TRANS comment may be needed for translators to know better what they have to translate.

Remember that today is UI freeze.

Changed 9 years ago by erikos

device attached

Changed 9 years ago by erikos

Error message with current code

Changed 9 years ago by erikos

Proposal

Changed 9 years ago by garycmartin

Mock-up suggestion

Changed 9 years ago by erikos

New patch that does address Tomeu's comments and a slightly modified design (after discussion with Gary)

Changed 9 years ago by erikos

Connect

Changed 9 years ago by erikos

Error: could not connect / false config

Changed 9 years ago by erikos

No connection set in the CP

comment:12 Changed 9 years ago by erikos

  • Keywords r? added; r! removed

comment:13 Changed 9 years ago by erikos

  • Cc garycmartin added

http://bugs.sugarlabs.org/attachment/ticket/1759/gsm_error_1.png

http://bugs.sugarlabs.org/attachment/ticket/1759/gsm_error_2.png

Gary, the code had two parts for the alert (I did not see that yesterday): So we have an error message and a suggestion what changes could help. I made the title (Error) bold and do place the suggestion below it. What do you think - is that visually acceptable? The rest I changed like we discussed yesterday.

comment:14 Changed 9 years ago by erikos

  • Keywords r+ added; r? removed
  • Resolution set to fixed
  • Status changed from new to closed

comment:15 Changed 9 years ago by erikos

Thanks Daniel for working on this! I thought I would champion it quickly so it does not miss another boat...

comment:16 Changed 9 years ago by Dcastelo

Thanks you for finish this issue !

comment:17 Changed 6 years ago by dnarvaez

  • Milestone 0.88.x deleted

Milestone 0.88.x deleted

Note: See TracTickets for help on using tickets.