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

WIP: Expose more information about shipments by PostNL #18334

Merged
merged 2 commits into from Jan 11, 2019

Conversation

Projects
None yet
7 participants
@basbl
Copy link
Contributor

commented Nov 9, 2018

Description:

Expose all known information about shipments in the shipments attribute.

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7445

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed:

@homeassistant

This comment has been minimized.

Copy link

commented Nov 9, 2018

Hi @basbl,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@ghost ghost added the in progress label Nov 9, 2018

@homeassistant homeassistant added cla-signed and removed cla-needed labels Nov 9, 2018

@basbl basbl referenced this pull request Nov 9, 2018

Merged

Changed how packages are exposed by the postnl sensor. #7445

2 of 2 tasks complete
@iMicknl

This comment has been minimized.

Copy link
Contributor

commented Nov 10, 2018

Hi @basbl! I created the original implementation of the PostNL component and I can understand that you would like to have more information about every shipment available in HomeAssistant.

self._api.parse_datetime(status, '%d-%m-%Y', '%H:%M') is necessary to convert the DateString to a human readable format, which you left out. Is this something you were planning to parse in a custom LoveLace component or in another way? I am wondering how you want to visualise all shipments in HomeAssistant after your edits.

I think the main point of this component is to show the amount of packages and when they will be delivered. What are your thoughts about this?

@peternijssen

This comment has been minimized.

Copy link
Contributor

commented Nov 10, 2018

Seems like a nice change. I think it actually allows you to get the shipments and do something with them (like sending a message).

I guess it kinda resolves this case;
https://gathering.tweakers.net/forum/list_message/55934285#55934285

@basbl

This comment has been minimized.

Copy link
Contributor Author

commented Nov 12, 2018

Hi @iMicknl! Thanks for your time and effort!

Like @peternijssen says I'd like to be able to do something with shipments. I'm planning to look into https://developers.home-assistant.io/docs/en/frontend_add_more_info.html as a solution for the display of the shipments.

Do you think this will be an adequate solution?

@basbl basbl changed the title Expose more information about shipments by PostNL WIP: Expose more information about shipments by PostNL Nov 12, 2018

@basbl

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

There doesn't seem to be a way to add a sensor specific "more info"-dialog, only domain wide, or did I miss something? Is there any other way to customise attribute formatting for a specific sensor in the dialog?

@balloob

This comment has been minimized.

Copy link
Member

commented Jan 11, 2019

It's not possible to add custom more info dialogs.

We should store data in the state machine for computer processing, not optimized for display on UI. Consider creating a custom lovelace card to display the data correctly.

@ghost ghost assigned balloob Jan 11, 2019

@balloob balloob merged commit 8c27bf8 into home-assistant:dev Jan 11, 2019

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
WIP work in progress
Details
Hound No violations found. Woof!

@ghost ghost removed the in progress label Jan 11, 2019

mxworm added a commit to mxworm/home-assistant that referenced this pull request Jan 12, 2019

Merge branch 'dev' into current
* dev: (150 commits)
  Update coveragerc file
  Upgrade greeneye_monitor to 1.0 (home-assistant#19631)
  Update doorbird events to include URLs on event_data (home-assistant#19262)
  Support for html5 notifications to suggest their names (home-assistant#19965)
  catch TypeError's in addition to ValueError's for unifi direct device tracker (home-assistant#19994)
  'latest_dir' referenced before assignment (home-assistant#19952)
  Repackage ZHA component (home-assistant#19989)
  Add support for HomeKit Controller Locks (home-assistant#19867)
  Don't set friendly_name in Zha entity. (home-assistant#19991)
  Lint
  Wink: Update pubnubsub-handler version to make it compatible with python 3.7 (home-assistant#19625)
  Upgrade pytest-cov to 2.6.1 (home-assistant#19988)
  Upgrade huawei-lte-api to 1.1.3 (home-assistant#19987)
  Support for multiple Fibaro gateways (home-assistant#19705)
  Split locative to a separate component (home-assistant#19964)
  Bumped version to 0.86.0.dev0
  Fix fail2ban tests
  Expose more information about shipments by PostNL (home-assistant#18334)
  Fix the anthemav component by removing a debugging line. (home-assistant#19979)
  Add support for 'via_hub' for device_info (home-assistant#19454)
  ...

# Conflicts:
#	homeassistant/components/homematicip_cloud/__init__.py
#	requirements_all.txt
#	requirements_test_all.txt

@balloob balloob referenced this pull request Jan 23, 2019

Merged

0.86.0 #20354

@TheLastProject

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2019

It seems this change broke the postNL sensor:
image

Is there any way to get this working again?

@iMicknl

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2019

I am also more a fan of my previous implementation, since status = self._api.parse_datetime(status, '%d-%m-%Y', '%H:%M') is also missing now. Which means that there is no human readable time information available anymore for the package deliveries.

However a custom LoveLace card will solve the output problem, but is there already a way to add a LoveLace card by default? Since this breaking change will break many interfaces.

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jan 25, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.