Adding USCIS component#13764
Conversation
| APP_RECEIPT_NUM = "appReceiptNum" | ||
| INIT_CASE_SEARCH = "initCaseSearch" | ||
| CASE_STATUS = "CHECK STATUS" | ||
| UPDATE_TEXT_XPATH = "/html/body/div[2]/form/div/div[1]" \ |
There was a problem hiding this comment.
This is too much protocol specific information and should be extracted into an external package.
|
Did you know that you can signup for a USCIS account and get notified by email/text of changes to your case? Also, we should check in the terms of the USCIS if they allow being scraped. If not, we should not merge this. |
|
@balloob I updated the component to use uscisstatus package.. also we check did check about the USCIS website term "Information presented on this WWW site is considered public information and may be distributed or copied. ".. i think it should be ready to merge |
| else: | ||
| _LOGGER.error("Setup USCIS Sensor Fail" | ||
| " check if your Case ID is Valid") | ||
| return |
| """Using Request to access USCIS website and fetch data.""" | ||
| import uscisstatus | ||
| try: | ||
| status, date = uscisstatus.get_case_status(self._case_id) |
There was a problem hiding this comment.
It's fine because it's a first version, but generally speaking returning a tuple will suck if you want to extend the returned data in the future. Namedtuples are your friend.
| status, date = uscisstatus.get_case_status(self._case_id) | ||
| self._attributes = { | ||
| self.CURRENT_STATUS: status, | ||
| self.LAST_CASE_UPDATE: date |
There was a problem hiding this comment.
This is already stored as state so shouldn't be an attribute.
|
@balloob changes has been made |
| """Return the state attributes.""" | ||
| return self._attributes | ||
|
|
||
| @Throttle(HOURS_TO_UPDATE) |
There was a problem hiding this comment.
Could you call the const MIN_TIME_BETWEEN_UPDATES and move the declaration to the top of the component? Just to be in sync with other sensors.
There was a problem hiding this comment.
I did name it like this because timedelta has hour=24so i thought would be better to have it as HOUR_TO_UPDATE instead but if you still think it's better to rename it just let me know @syssi
There was a problem hiding this comment.
@meauxt We use MIN_TIME_BETWEEN_UPDATES everywhere and that can accommodate any time units (hours, minutes, seconds). It may be better, for the sake of consistency, to use the same label.
| self._attributes = { | ||
| self.CURRENT_STATUS: status['status'] | ||
| } | ||
| self._state = status['date'] |
There was a problem hiding this comment.
Why do you return the date as state? The status looks like more important to me. I don't know USCIS. ;-) Could you provide some example output here?
There was a problem hiding this comment.
@syssi The date indicates last update. The actual status is quite verbose and beyond the 255 chars limit. More importantly, the status update text may not be very useful for automation. Here's an example of a status message:
On March 8, 2018, we accepted the fingerprint fee for your Form I-485, Application to Register Permanent Residence or Adjust Status, Receipt Number SRCxyz. Our Texas Service Center location is working on your case. We mailed you a notice describing how we will process your case. Please follow the instructions in the notice. If you move, go to USCIS website to give us your new mailing address.
Description:
This component will fetch the lastest status on case from USCIS on daily basis
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5138>
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
REQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices: